[PATCH v1] io_uring: Fix memory leak if file setup fails.

[PATCH v1] io_uring: Fix memory leak if file setup fails.

[Noah Goldstein]

If `get_unused_fd_flags` files fails (either in setting up `ctx` as
`tctx->last` or `get_unused_fd_flags`) `ctx` will never be freed.

Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
---
I very well may be missing something (or there may be a double
free if the failure is after `get_unused_fd_flags`) but looks
to me to be a memory leak.
 fs/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index a3b76e63f9da..9685a7be48e3 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -11863,7 +11863,7 @@ static __cold int io_uring_create(unsigned entries, struct io_uring_params *p,
 	if (ret < 0) {
 		/* fput will clean it up */
 		fput(file);
-		return ret;
+		goto err;
 	}
 
 	trace_io_uring_create(ret, ctx, p->sq_entries, p->cq_entries, p->flags);
-- 
2.25.1

Re: [PATCH v1] io_uring: Fix memory leak if file setup fails.

[Jens Axboe]

On 4/28/22 6:42 PM, Noah Goldstein wrote:
> If `get_unused_fd_flags` files fails (either in setting up `ctx` as
> `tctx->last` or `get_unused_fd_flags`) `ctx` will never be freed.

There's a comment there telling you why, the fput will end up
releasing it just like it would when an application closes it.

> I very well may be missing something (or there may be a double
> free if the failure is after `get_unused_fd_flags`) but looks
> to me to be a memory leak.

Have you tried synthetically reproducing the two failures you're
thinking of and tracing cleanup?

-- 
Jens Axboe

Re: [PATCH v1] io_uring: Fix memory leak if file setup fails.

[Noah Goldstein]

On Fri, Apr 29, 2022 at 7:47 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 4/28/22 6:42 PM, Noah Goldstein wrote:
> > If `get_unused_fd_flags` files fails (either in setting up `ctx` as
> > `tctx->last` or `get_unused_fd_flags`) `ctx` will never be freed.
>
> There's a comment there telling you why, the fput will end up
> releasing it just like it would when an application closes it.

I see. Thought the 'it' refered to in the comment was the file, not
ctx.