| Message ID | 20220524213727.409630-6-axboe@kernel.dk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Misc cleanups | expand |
On Tue, May 24, 2022 at 03:37:26PM -0600, Jens Axboe wrote: >If the opcode only stores data that needs to be kfree'ed in >req->async_data, then it doesn't need special handling in >our cleanup handler. > >This has the added bonus of removing knowledge of those kinds of >special async_data to the io_uring core. > >Signed-off-by: Jens Axboe <axboe@kernel.dk> >--- > fs/io_uring.c | 18 ------------------ > 1 file changed, 18 deletions(-) > >diff --git a/fs/io_uring.c b/fs/io_uring.c >index 408265a03563..8188c47956ad 100644 >--- a/fs/io_uring.c >+++ b/fs/io_uring.c >@@ -8229,24 +8229,6 @@ static void io_clean_op(struct io_kiocb *req) > > if (req->flags & REQ_F_NEED_CLEANUP) { > switch (req->opcode) { >- case IORING_OP_READV: >- case IORING_OP_READ_FIXED: >- case IORING_OP_READ: >- case IORING_OP_WRITEV: >- case IORING_OP_WRITE_FIXED: >- case IORING_OP_WRITE: { >- struct io_async_rw *io = req->async_data; >- >- kfree(io->free_iovec); Removing this kfree may cause a leak. For READV/WRITEV atleast, io->free_iovec will hold the address of allocated iovec array if input was larger than UIO_FASTIOV.
On 5/25/22 2:46 AM, Kanchan Joshi wrote: > On Tue, May 24, 2022 at 03:37:26PM -0600, Jens Axboe wrote: >> If the opcode only stores data that needs to be kfree'ed in >> req->async_data, then it doesn't need special handling in >> our cleanup handler. >> >> This has the added bonus of removing knowledge of those kinds of >> special async_data to the io_uring core. >> >> Signed-off-by: Jens Axboe <axboe@kernel.dk> >> --- >> fs/io_uring.c | 18 ------------------ >> 1 file changed, 18 deletions(-) >> >> diff --git a/fs/io_uring.c b/fs/io_uring.c >> index 408265a03563..8188c47956ad 100644 >> --- a/fs/io_uring.c >> +++ b/fs/io_uring.c >> @@ -8229,24 +8229,6 @@ static void io_clean_op(struct io_kiocb *req) >> >> if (req->flags & REQ_F_NEED_CLEANUP) { >> switch (req->opcode) { >> - case IORING_OP_READV: >> - case IORING_OP_READ_FIXED: >> - case IORING_OP_READ: >> - case IORING_OP_WRITEV: >> - case IORING_OP_WRITE_FIXED: >> - case IORING_OP_WRITE: { >> - struct io_async_rw *io = req->async_data; >> - >> - kfree(io->free_iovec); > > Removing this kfree may cause a leak. > For READV/WRITEV atleast, io->free_iovec will hold the address of > allocated iovec array if input was larger than UIO_FASTIOV. I think I was too tired when I did and saw it freeing ->async_data, which is obviously not the case. I'll drop this one.
diff --git a/fs/io_uring.c b/fs/io_uring.c index 408265a03563..8188c47956ad 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -8229,24 +8229,6 @@ static void io_clean_op(struct io_kiocb *req) if (req->flags & REQ_F_NEED_CLEANUP) { switch (req->opcode) { - case IORING_OP_READV: - case IORING_OP_READ_FIXED: - case IORING_OP_READ: - case IORING_OP_WRITEV: - case IORING_OP_WRITE_FIXED: - case IORING_OP_WRITE: { - struct io_async_rw *io = req->async_data; - - kfree(io->free_iovec); - break; - } - case IORING_OP_RECVMSG: - case IORING_OP_SENDMSG: { - struct io_async_msghdr *io = req->async_data; - - kfree(io->free_iov); - break; - } case IORING_OP_OPENAT: case IORING_OP_OPENAT2: if (req->open.filename)
If the opcode only stores data that needs to be kfree'ed in req->async_data, then it doesn't need special handling in our cleanup handler. This has the added bonus of removing knowledge of those kinds of special async_data to the io_uring core. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- fs/io_uring.c | 18 ------------------ 1 file changed, 18 deletions(-)