public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH] tools/io_uring: Fix missing check for return value of malloc()
@ 2023-06-15 12:50 Chenyuan Mi
  2023-06-15 13:00 ` Ammar Faizi
  0 siblings, 1 reply; 5+ messages in thread
From: Chenyuan Mi @ 2023-06-15 12:50 UTC (permalink / raw)
  To: axboe; +Cc: sml.silence, io-uring, linux-kernel, Chenyuan Mi

The malloc() function may return NULL when it fails,
which may cause null pointer deference in kmalloc(),
add Null check for return value of malloc().

Found by our static analysis tool.

Signed-off-by: Chenyuan Mi <[email protected]>
---
 tools/io_uring/io_uring-bench.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/io_uring/io_uring-bench.c b/tools/io_uring/io_uring-bench.c
index 7703f0118385..a7fedfdb9b84 100644
--- a/tools/io_uring/io_uring-bench.c
+++ b/tools/io_uring/io_uring-bench.c
@@ -560,6 +560,11 @@ int main(int argc, char *argv[])
 	pthread_create(&s->thread, NULL, submitter_fn, s);
 
 	fdepths = malloc(8 * s->nr_files);
+	if (!fdepths) {
+		printf("malloc failed");
+		return 1;
+	}
+
 	reap = calls = done = 0;
 	do {
 		unsigned long this_done = 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] tools/io_uring: Fix missing check for return value of malloc()
@ 2023-06-15 12:53 Chenyuan Mi
  0 siblings, 0 replies; 5+ messages in thread
From: Chenyuan Mi @ 2023-06-15 12:53 UTC (permalink / raw)
  To: axboe; +Cc: sml.silence, io-uring, linux-kernel, Chenyuan Mi

The malloc() function may return NULL when it fails,
which may cause null pointer deference. Add Null 
check for return value of malloc().

Found by our static analysis tool.

Signed-off-by: Chenyuan Mi <[email protected]>
---
 tools/io_uring/io_uring-bench.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/io_uring/io_uring-bench.c b/tools/io_uring/io_uring-bench.c
index 7703f0118385..a7fedfdb9b84 100644
--- a/tools/io_uring/io_uring-bench.c
+++ b/tools/io_uring/io_uring-bench.c
@@ -560,6 +560,11 @@ int main(int argc, char *argv[])
 	pthread_create(&s->thread, NULL, submitter_fn, s);
 
 	fdepths = malloc(8 * s->nr_files);
+	if (!fdepths) {
+		printf("malloc failed");
+		return 1;
+	}
+
 	reap = calls = done = 0;
 	do {
 		unsigned long this_done = 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] tools/io_uring: Fix missing check for return value of malloc()
  2023-06-15 12:50 [PATCH] tools/io_uring: Fix missing check for return value of malloc() Chenyuan Mi
@ 2023-06-15 13:00 ` Ammar Faizi
  2023-06-15 13:13   ` Jens Axboe
  0 siblings, 1 reply; 5+ messages in thread
From: Ammar Faizi @ 2023-06-15 13:00 UTC (permalink / raw)
  To: Chenyuan Mi, Jens Axboe
  Cc: Pavel Begunkov, io-uring Mailing List, Linux Kernel Mailing List

On 6/15/23 7:50 PM, Chenyuan Mi wrote:
> The malloc() function may return NULL when it fails,
> which may cause null pointer deference in kmalloc(),

It's a userspace app, there is no kmalloc(). Also, I don't think it's
worth to fix a missing ENOMEM handling for that old test program. But
anyway, let's wait for maintainers' comment on this.

-- 
Ammar Faizi


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tools/io_uring: Fix missing check for return value of malloc()
  2023-06-15 13:00 ` Ammar Faizi
@ 2023-06-15 13:13   ` Jens Axboe
       [not found]     ` <[email protected]>
  0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2023-06-15 13:13 UTC (permalink / raw)
  To: Ammar Faizi, Chenyuan Mi
  Cc: Pavel Begunkov, io-uring Mailing List, Linux Kernel Mailing List

On 6/15/23 7:00?AM, Ammar Faizi wrote:
> On 6/15/23 7:50 PM, Chenyuan Mi wrote:
>> The malloc() function may return NULL when it fails,
>> which may cause null pointer deference in kmalloc(),
> 
> It's a userspace app, there is no kmalloc(). Also, I don't think it's
> worth to fix a missing ENOMEM handling for that old test program. But
> anyway, let's wait for maintainers' comment on this.

Definitely not worth it, and I find it odd how the author would target
just one of multiple allocations in that file. I'm guessing it's because
this checker only checks for malloc(), and no thought has otherwise gone
into a) if the patch makes any sense at all, and b) if it does make
sense, are there potentially other cases to consider?

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tools/io uring: Fix missing check for return value of malloc()
       [not found]     ` <[email protected]>
@ 2023-06-15 13:37       ` Jens Axboe
  0 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2023-06-15 13:37 UTC (permalink / raw)
  To: cymi20
  Cc: [email protected], [email protected],
	[email protected], [email protected]

On 6/15/23 7:26?AM, cymi20 wrote:
> Actually this checker is driven by inconsistency, it find almost all
> callsite of malloc() in this module has Null check, except this
> callsite.

1) don't top post
2) don't send html emails

But more importantly, actually check the code before making wrong
statements like that.

The patch is pointless.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-06-15 13:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-15 12:50 [PATCH] tools/io_uring: Fix missing check for return value of malloc() Chenyuan Mi
2023-06-15 13:00 ` Ammar Faizi
2023-06-15 13:13   ` Jens Axboe
     [not found]     ` <[email protected]>
2023-06-15 13:37       ` [PATCH] tools/io uring: " Jens Axboe
  -- strict thread matches above, loose matches on Subject: below --
2023-06-15 12:53 [PATCH] tools/io_uring: " Chenyuan Mi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox