public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH] io_uring: fix memleak in io_init_wq_offload()
@ 2021-07-20  8:38 Yang Yingliang
  2021-07-20 10:16 ` Pavel Begunkov
  2021-07-20 13:52 ` Jens Axboe
  0 siblings, 2 replies; 3+ messages in thread
From: Yang Yingliang @ 2021-07-20  8:38 UTC (permalink / raw)
  To: linux-kernel, io-uring; +Cc: axboe, asml.silence

I got memory leak report when doing fuzz test:

BUG: memory leak
unreferenced object 0xffff888107310a80 (size 96):
comm "syz-executor.6", pid 4610, jiffies 4295140240 (age 20.135s)
hex dump (first 32 bytes):
01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N..........
backtrace:
[<000000001974933b>] kmalloc include/linux/slab.h:591 [inline]
[<000000001974933b>] kzalloc include/linux/slab.h:721 [inline]
[<000000001974933b>] io_init_wq_offload fs/io_uring.c:7920 [inline]
[<000000001974933b>] io_uring_alloc_task_context+0x466/0x640 fs/io_uring.c:7955
[<0000000039d0800d>] __io_uring_add_tctx_node+0x256/0x360 fs/io_uring.c:9016
[<000000008482e78c>] io_uring_add_tctx_node fs/io_uring.c:9052 [inline]
[<000000008482e78c>] __do_sys_io_uring_enter fs/io_uring.c:9354 [inline]
[<000000008482e78c>] __se_sys_io_uring_enter fs/io_uring.c:9301 [inline]
[<000000008482e78c>] __x64_sys_io_uring_enter+0xabc/0xc20 fs/io_uring.c:9301
[<00000000b875f18f>] do_syscall_x64 arch/x86/entry/common.c:50 [inline]
[<00000000b875f18f>] do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
[<000000006b0a8484>] entry_SYSCALL_64_after_hwframe+0x44/0xae

CPU0                          CPU1
io_uring_enter                io_uring_enter
io_uring_add_tctx_node        io_uring_add_tctx_node
__io_uring_add_tctx_node      __io_uring_add_tctx_node
io_uring_alloc_task_context   io_uring_alloc_task_context
io_init_wq_offload            io_init_wq_offload
hash = kzalloc                hash = kzalloc
ctx->hash_map = hash          ctx->hash_map = hash <- one of the hash is leaked

When calling io_uring_enter() in parallel, the 'hash_map' will be leaked, 
add uring_lock to protect 'hash_map'.

Fixes: e941894eae31 ("io-wq: make buffered file write hashed work map per-ctx")
Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Yang Yingliang <[email protected]>
---
 fs/io_uring.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 0cac361bf6b8..63d3a9c2a2a6 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7899,15 +7899,19 @@ static struct io_wq *io_init_wq_offload(struct io_ring_ctx *ctx,
 	struct io_wq_data data;
 	unsigned int concurrency;
 
+	mutex_lock(&ctx->uring_lock);
 	hash = ctx->hash_map;
 	if (!hash) {
 		hash = kzalloc(sizeof(*hash), GFP_KERNEL);
-		if (!hash)
+		if (!hash) {
+			mutex_unlock(&ctx->uring_lock);
 			return ERR_PTR(-ENOMEM);
+		}
 		refcount_set(&hash->refs, 1);
 		init_waitqueue_head(&hash->wait);
 		ctx->hash_map = hash;
 	}
+	mutex_unlock(&ctx->uring_lock);
 
 	data.hash = hash;
 	data.task = task;
-- 
2.25.1


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

* Re: [PATCH] io_uring: fix memleak in io_init_wq_offload()
  2021-07-20  8:38 [PATCH] io_uring: fix memleak in io_init_wq_offload() Yang Yingliang
@ 2021-07-20 10:16 ` Pavel Begunkov
  2021-07-20 13:52 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: Pavel Begunkov @ 2021-07-20 10:16 UTC (permalink / raw)
  To: Yang Yingliang, linux-kernel, io-uring; +Cc: axboe

On 7/20/21 9:38 AM, Yang Yingliang wrote:
> I got memory leak report when doing fuzz test:

Reviewed-by: Pavel Begunkov <[email protected]>
Cc: [email protected]

> 
> BUG: memory leak
> unreferenced object 0xffff888107310a80 (size 96):
> comm "syz-executor.6", pid 4610, jiffies 4295140240 (age 20.135s)
> hex dump (first 32 bytes):
> 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N..........
> backtrace:
> [<000000001974933b>] kmalloc include/linux/slab.h:591 [inline]
> [<000000001974933b>] kzalloc include/linux/slab.h:721 [inline]
> [<000000001974933b>] io_init_wq_offload fs/io_uring.c:7920 [inline]
> [<000000001974933b>] io_uring_alloc_task_context+0x466/0x640 fs/io_uring.c:7955
> [<0000000039d0800d>] __io_uring_add_tctx_node+0x256/0x360 fs/io_uring.c:9016
> [<000000008482e78c>] io_uring_add_tctx_node fs/io_uring.c:9052 [inline]
> [<000000008482e78c>] __do_sys_io_uring_enter fs/io_uring.c:9354 [inline]
> [<000000008482e78c>] __se_sys_io_uring_enter fs/io_uring.c:9301 [inline]
> [<000000008482e78c>] __x64_sys_io_uring_enter+0xabc/0xc20 fs/io_uring.c:9301
> [<00000000b875f18f>] do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> [<00000000b875f18f>] do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
> [<000000006b0a8484>] entry_SYSCALL_64_after_hwframe+0x44/0xae
> 
> CPU0                          CPU1
> io_uring_enter                io_uring_enter
> io_uring_add_tctx_node        io_uring_add_tctx_node
> __io_uring_add_tctx_node      __io_uring_add_tctx_node
> io_uring_alloc_task_context   io_uring_alloc_task_context
> io_init_wq_offload            io_init_wq_offload
> hash = kzalloc                hash = kzalloc
> ctx->hash_map = hash          ctx->hash_map = hash <- one of the hash is leaked
> 
> When calling io_uring_enter() in parallel, the 'hash_map' will be leaked, 
> add uring_lock to protect 'hash_map'.
> 
> Fixes: e941894eae31 ("io-wq: make buffered file write hashed work map per-ctx")
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: Yang Yingliang <[email protected]>
> ---
>  fs/io_uring.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 0cac361bf6b8..63d3a9c2a2a6 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -7899,15 +7899,19 @@ static struct io_wq *io_init_wq_offload(struct io_ring_ctx *ctx,
>  	struct io_wq_data data;
>  	unsigned int concurrency;
>  
> +	mutex_lock(&ctx->uring_lock);
>  	hash = ctx->hash_map;
>  	if (!hash) {
>  		hash = kzalloc(sizeof(*hash), GFP_KERNEL);
> -		if (!hash)
> +		if (!hash) {
> +			mutex_unlock(&ctx->uring_lock);
>  			return ERR_PTR(-ENOMEM);
> +		}
>  		refcount_set(&hash->refs, 1);
>  		init_waitqueue_head(&hash->wait);
>  		ctx->hash_map = hash;
>  	}
> +	mutex_unlock(&ctx->uring_lock);
>  
>  	data.hash = hash;
>  	data.task = task;
> 

-- 
Pavel Begunkov

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

* Re: [PATCH] io_uring: fix memleak in io_init_wq_offload()
  2021-07-20  8:38 [PATCH] io_uring: fix memleak in io_init_wq_offload() Yang Yingliang
  2021-07-20 10:16 ` Pavel Begunkov
@ 2021-07-20 13:52 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2021-07-20 13:52 UTC (permalink / raw)
  To: Yang Yingliang, linux-kernel, io-uring; +Cc: asml.silence

On 7/20/21 2:38 AM, Yang Yingliang wrote:
> I got memory leak report when doing fuzz test:
> 
> BUG: memory leak
> unreferenced object 0xffff888107310a80 (size 96):
> comm "syz-executor.6", pid 4610, jiffies 4295140240 (age 20.135s)
> hex dump (first 32 bytes):
> 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00 .....N..........
> backtrace:
> [<000000001974933b>] kmalloc include/linux/slab.h:591 [inline]
> [<000000001974933b>] kzalloc include/linux/slab.h:721 [inline]
> [<000000001974933b>] io_init_wq_offload fs/io_uring.c:7920 [inline]
> [<000000001974933b>] io_uring_alloc_task_context+0x466/0x640 fs/io_uring.c:7955
> [<0000000039d0800d>] __io_uring_add_tctx_node+0x256/0x360 fs/io_uring.c:9016
> [<000000008482e78c>] io_uring_add_tctx_node fs/io_uring.c:9052 [inline]
> [<000000008482e78c>] __do_sys_io_uring_enter fs/io_uring.c:9354 [inline]
> [<000000008482e78c>] __se_sys_io_uring_enter fs/io_uring.c:9301 [inline]
> [<000000008482e78c>] __x64_sys_io_uring_enter+0xabc/0xc20 fs/io_uring.c:9301
> [<00000000b875f18f>] do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> [<00000000b875f18f>] do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
> [<000000006b0a8484>] entry_SYSCALL_64_after_hwframe+0x44/0xae
> 
> CPU0                          CPU1
> io_uring_enter                io_uring_enter
> io_uring_add_tctx_node        io_uring_add_tctx_node
> __io_uring_add_tctx_node      __io_uring_add_tctx_node
> io_uring_alloc_task_context   io_uring_alloc_task_context
> io_init_wq_offload            io_init_wq_offload
> hash = kzalloc                hash = kzalloc
> ctx->hash_map = hash          ctx->hash_map = hash <- one of the hash is leaked
> 
> When calling io_uring_enter() in parallel, the 'hash_map' will be leaked, 
> add uring_lock to protect 'hash_map'.

Good catch! Applied, thanks.

-- 
Jens Axboe


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

end of thread, other threads:[~2021-07-20 13:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-20  8:38 [PATCH] io_uring: fix memleak in io_init_wq_offload() Yang Yingliang
2021-07-20 10:16 ` Pavel Begunkov
2021-07-20 13:52 ` Jens Axboe

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