io_uring: fix a use after free in io_async_task_func()
Subsystems (3)
PATCH 1/1 io_uring: fix a use after free in io_async_task_func()
The "apoll" variable is freed and then used on the next line. We need to move the free down a few lines.
--- fs/io_uring.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
fs/io_uring.c +2 -1
@@ -4652,12 +4652,13 @@ static void io_async_task_func(struct callback_head *cb)/* restore ->work in case we need to retry again */ if (req->flags &REQ_F_WORK_INITIALIZED )memcpy (&req->work, &apoll->work,sizeof (req->work)); -kfree (apoll);if (!READ_ONCE (apoll->poll.canceled))__io_req_task_submit (req);else __io_req_task_cancel (req, -ECANCELED ); + +kfree (apoll); } static int io_async_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
Signature · 1 line
2.27.0
Quoted 2 lines · Dan Carpenter
The "apoll" variable is freed and then used on the next line. We need to move the free down a few lines.
My bad, thanks for spotting that!
A comment below
Quoted 12 lines
Fixes 0be0b0e33b0b ("io_uring: simplify io_async_task_func()")Signed-off-by Dan Carpenter <[email protected]>--- fs/io_uring.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)fs/io_uring.c +0
-0
fs/io_uring.c static void io_async_task_func(struct callback_head *cb) · line 4655
Quoted 4 lines
fs/io_uring.c +0
-1@@ -4652,12 +4652,13 @@ static void io_async_task_func(struct callback_head *cb)/* restore ->work in case we need to retry again */ if (req->flags &REQ_F_WORK_INITIALIZED )memcpy (&req->work, &apoll->work,sizeof (req->work)); -kfree (apoll);
__io_req_task_submit() can take some time, e.g. waiting for a mutex, so I'd rather free it before, but save req->poll.canceled in a local var.
Quoted 11 lines
fs/io_uring.c +2
-0if (!READ_ONCE (apoll->poll.canceled))__io_req_task_submit (req);else __io_req_task_cancel (req, -ECANCELED ); + +kfree (apoll); } static int io_async_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
Signature · 1 line
Pavel Begunkov
Jens Axboe
maintainerQuoted 14 lines · Pavel Begunkov
Quoted 2 lines · Dan Carpenter
The "apoll" variable is freed and then used on the next line. We need to move the free down a few lines.
My bad, thanks for spotting that!
A comment belowQuoted 7 lines
Fixes 0be0b0e33b0b ("io_uring: simplify io_async_task_func()")Signed-off-by Dan Carpenter <[email protected]>--- fs/io_uring.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Quoted 12 lines
fs/io_uring.c static void io_async_task_func(struct callback_head *cb) · line 4655
Quoted 9 lines
fs/io_uring.c +0
-1@@ -4652,12 +4652,13 @@ static void io_async_task_func(struct callback_head *cb)/* restore ->work in case we need to retry again */ if (req->flags &REQ_F_WORK_INITIALIZED )memcpy (&req->work, &apoll->work,sizeof (req->work)); -kfree (apoll);
__io_req_task_submit()can take some time, e.g. waiting for a mutex, so I'd rather free it before, but savereq->poll.canceledin a local var.
It's 64 bytes of data, really doesn't matter. Let's just keep it simple.
Signature · 1 line
Jens Axboe
Jens Axboe
maintainerQuoted 2 lines · Dan Carpenter
The "apoll" variable is freed and then used on the next line. We need to move the free down a few lines.
Thanks for spotting this Dan, applied.
Signature · 1 line
Jens Axboe
Quoted 5 lines · Jens Axboe
Quoted 2 lines · Dan Carpenter
The "apoll" variable is freed and then used on the next line. We need to move the free down a few lines.
Thanks for spotting this Dan, applied.
I wonder why gcc can't find it... It shouldn't be hard to do after marking free-like functions with an attribute.
Are there such tools for the kernel?
Signature · 1 line
Pavel Begunkov
Quoted 11 lines · Pavel Begunkov
Quoted 5 lines · Jens Axboe
Quoted 2 lines · Dan Carpenter
The "apoll" variable is freed and then used on the next line. We need to move the free down a few lines.
Thanks for spotting this Dan, applied.
I wonder why gcc can't find it... It shouldn't be hard to do after marking free-like functions with an attribute.
Are there such tools for the kernel?
GCC doesn't have an __attribute__((free)) yet. Martin Sebor is working on
it: https://gcc.gnu.org/bugzilla/show_bug.cgi?id‡736
also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id”527
(I just confirmed with him on IRC that he's still working on it; it's part of an ongoing larger project)
Quoted 11 lines · Matthew Wilcox
Quoted 10 lines · Pavel Begunkov
Quoted 5 lines · Jens Axboe
Quoted 2 lines · Dan Carpenter
The "apoll" variable is freed and then used on the next line. We need to move the free down a few lines.
Thanks for spotting this Dan, applied.
I wonder why gcc can't find it... It shouldn't be hard to do after marking free-like functions with an attribute.
Quoted 8 lines
Quoted 1 line
Are there such tools for the kernel?
GCC doesn't have an __attribute__((free)) yet. Martin Sebor is working on
it: https://gcc.gnu.org/bugzilla/show_bug.cgi?id‡736
also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id”527(I just confirmed with him on IRC that he's still working on it; it's part of an ongoing larger project)
Good to know. It looks broader indeed, hence won't probably be here too soon.
Signature · 1 line
Pavel Begunkov