io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] io_uring: ensure open/openat2 name is cleaned on cancelation
@ 2020-09-24 20:59 Jens Axboe
  2020-09-25  8:32 ` Stefano Garzarella
  0 siblings, 1 reply; 3+ messages in thread
From: Jens Axboe @ 2020-09-24 20:59 UTC (permalink / raw)
  To: io-uring

io_uring: ensure open/openat2 name is cleaned on cancelation

If we cancel these requests, we'll leak the memory associated with the
filename. Add them to the table of ops that need cleaning, if
REQ_F_NEED_CLEANUP is set.

Cc: stable@vger.kernel.org # v5.6+
Signed-off-by: Jens Axboe <axboe@kernel.dk>

--

diff --git a/fs/io_uring.c b/fs/io_uring.c
index e6004b92e553..0ab16df31288 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5671,6 +5671,11 @@ static void __io_clean_op(struct io_kiocb *req)
 			io_put_file(req, req->splice.file_in,
 				    (req->splice.flags & SPLICE_F_FD_IN_FIXED));
 			break;
+		case IORING_OP_OPENAT:
+		case IORING_OP_OPENAT2:
+			if (req->open.filename)
+				putname(req->open.filename);
+			break;
 		}
 		req->flags &= ~REQ_F_NEED_CLEANUP;
 	}

-- 
Jens Axboe


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

* Re: [PATCH] io_uring: ensure open/openat2 name is cleaned on cancelation
  2020-09-24 20:59 [PATCH] io_uring: ensure open/openat2 name is cleaned on cancelation Jens Axboe
@ 2020-09-25  8:32 ` Stefano Garzarella
  2020-09-25 13:43   ` Jens Axboe
  0 siblings, 1 reply; 3+ messages in thread
From: Stefano Garzarella @ 2020-09-25  8:32 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring

On Thu, Sep 24, 2020 at 02:59:33PM -0600, Jens Axboe wrote:
> io_uring: ensure open/openat2 name is cleaned on cancelation
> 
> If we cancel these requests, we'll leak the memory associated with the
> filename. Add them to the table of ops that need cleaning, if
> REQ_F_NEED_CLEANUP is set.
> 

IIUC we inadvertently removed 'putname(req->open.filename)' from the cleanup
function in commit e62753e4e292 ("io_uring: call statx directly").

Should we add the Fixes tag?

    Fixes: e62753e4e292 ("io_uring: call statx directly")

I'm not sure since the code is changed a bit.

Anyway the patch LGTM:

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks,
Stefano

> Cc: stable@vger.kernel.org # v5.6+
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> --
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index e6004b92e553..0ab16df31288 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -5671,6 +5671,11 @@ static void __io_clean_op(struct io_kiocb *req)
>  			io_put_file(req, req->splice.file_in,
>  				    (req->splice.flags & SPLICE_F_FD_IN_FIXED));
>  			break;
> +		case IORING_OP_OPENAT:
> +		case IORING_OP_OPENAT2:
> +			if (req->open.filename)
> +				putname(req->open.filename);
> +			break;
>  		}
>  		req->flags &= ~REQ_F_NEED_CLEANUP;
>  	}
> 
> -- 
> Jens Axboe
> 


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

* Re: [PATCH] io_uring: ensure open/openat2 name is cleaned on cancelation
  2020-09-25  8:32 ` Stefano Garzarella
@ 2020-09-25 13:43   ` Jens Axboe
  0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2020-09-25 13:43 UTC (permalink / raw)
  To: Stefano Garzarella; +Cc: io-uring

On 9/25/20 2:32 AM, Stefano Garzarella wrote:
> On Thu, Sep 24, 2020 at 02:59:33PM -0600, Jens Axboe wrote:
>> io_uring: ensure open/openat2 name is cleaned on cancelation
>>
>> If we cancel these requests, we'll leak the memory associated with the
>> filename. Add them to the table of ops that need cleaning, if
>> REQ_F_NEED_CLEANUP is set.
>>
> 
> IIUC we inadvertently removed 'putname(req->open.filename)' from the cleanup
> function in commit e62753e4e292 ("io_uring: call statx directly").
> 
> Should we add the Fixes tag?
> 
>     Fixes: e62753e4e292 ("io_uring: call statx directly")

You are right, I got a bit tricked by it since that commit removed
the putname(), and then later on we got rid of the (now) empty
openat/openat2 entries.

I'll add the fixes, which means it's 5.8 only.

> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Added, thanks for reviewing!

-- 
Jens Axboe


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

end of thread, other threads:[~2020-09-25 13:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-24 20:59 [PATCH] io_uring: ensure open/openat2 name is cleaned on cancelation Jens Axboe
2020-09-25  8:32 ` Stefano Garzarella
2020-09-25 13:43   ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).