[PATCH] nvmet: fix a memory leak in controller identify

[PATCH] nvmet: fix a memory leak in controller identify

[Sagi Grimberg]

Simply free an allocated buffer once we copied its content
to the request sgl.

kmemleak complaint:
unreferenced object 0xffff8cd40c388000 (size 4096):
  comm "kworker/2:2H", pid 14739, jiffies 4401313113
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace (crc 0):
    [<ffffffff9e01087a>] kmemleak_alloc+0x4a/0x90
    [<ffffffff9d30324a>] __kmalloc_cache_noprof+0x35a/0x420
    [<ffffffffc180b0e2>] nvmet_execute_identify+0x912/0x9f0 [nvmet]
    [<ffffffffc181a72c>] nvmet_tcp_try_recv_pdu+0x84c/0xc90 [nvmet_tcp]
    [<ffffffffc181ac02>] nvmet_tcp_io_work+0x82/0x8b0 [nvmet_tcp]
    [<ffffffff9cfa7158>] process_one_work+0x178/0x3e0
    [<ffffffff9cfa8e9c>] worker_thread+0x2ec/0x420
    [<ffffffff9cfb2140>] kthread+0xf0/0x120
    [<ffffffff9cee36a4>] ret_from_fork+0x44/0x70
    [<ffffffff9ce7fdda>] ret_from_fork_asm+0x1a/0x30

Fixes: 84909f7decbd ("nvmet: use kzalloc instead of ZERO_PAGE in
nvme_execute_identify_ns_nvm()")
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/nvme/target/admin-cmd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index e670dc185a96..acc138bbf8f2 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -1068,6 +1068,7 @@ static void nvme_execute_identify_ns_nvm(struct nvmet_req *req)
 		goto out;
 	}
 	status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
+	kfree(id);
 out:
 	nvmet_req_complete(req, status);
 }
-- 
2.43.0

Re: [PATCH] nvmet: fix a memory leak in controller identify

[Nilay Shroff]

On 1/24/25 4:30 PM, Sagi Grimberg wrote:
> Simply free an allocated buffer once we copied its content
> to the request sgl.
> 
> kmemleak complaint:
> unreferenced object 0xffff8cd40c388000 (size 4096):
>   comm "kworker/2:2H", pid 14739, jiffies 4401313113
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace (crc 0):
>     [<ffffffff9e01087a>] kmemleak_alloc+0x4a/0x90
>     [<ffffffff9d30324a>] __kmalloc_cache_noprof+0x35a/0x420
>     [<ffffffffc180b0e2>] nvmet_execute_identify+0x912/0x9f0 [nvmet]
>     [<ffffffffc181a72c>] nvmet_tcp_try_recv_pdu+0x84c/0xc90 [nvmet_tcp]
>     [<ffffffffc181ac02>] nvmet_tcp_io_work+0x82/0x8b0 [nvmet_tcp]
>     [<ffffffff9cfa7158>] process_one_work+0x178/0x3e0
>     [<ffffffff9cfa8e9c>] worker_thread+0x2ec/0x420
>     [<ffffffff9cfb2140>] kthread+0xf0/0x120
>     [<ffffffff9cee36a4>] ret_from_fork+0x44/0x70
>     [<ffffffff9ce7fdda>] ret_from_fork_asm+0x1a/0x30
> 
> Fixes: 84909f7decbd ("nvmet: use kzalloc instead of ZERO_PAGE in
> nvme_execute_identify_ns_nvm()")
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
> ---
>  drivers/nvme/target/admin-cmd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index e670dc185a96..acc138bbf8f2 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -1068,6 +1068,7 @@ static void nvme_execute_identify_ns_nvm(struct nvmet_req *req)
>  		goto out;
>  	}
>  	status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
> +	kfree(id);
>  out:
>  	nvmet_req_complete(req, status);
>  }

Looks good to me.

Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>

Re: [PATCH] nvmet: fix a memory leak in controller identify

[Keith Busch]

On Fri, Jan 24, 2025 at 01:00:33PM +0200, Sagi Grimberg wrote:
> Simply free an allocated buffer once we copied its content
> to the request sgl.

Thanks, applied to nvme-6.14.

Re: [PATCH] nvmet: fix a memory leak in controller identify

[Hannes Reinecke]

On 1/24/25 12:00, Sagi Grimberg wrote:
> Simply free an allocated buffer once we copied its content
> to the request sgl.
> 
> kmemleak complaint:
> unreferenced object 0xffff8cd40c388000 (size 4096):
>    comm "kworker/2:2H", pid 14739, jiffies 4401313113
>    hex dump (first 32 bytes):
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace (crc 0):
>      [<ffffffff9e01087a>] kmemleak_alloc+0x4a/0x90
>      [<ffffffff9d30324a>] __kmalloc_cache_noprof+0x35a/0x420
>      [<ffffffffc180b0e2>] nvmet_execute_identify+0x912/0x9f0 [nvmet]
>      [<ffffffffc181a72c>] nvmet_tcp_try_recv_pdu+0x84c/0xc90 [nvmet_tcp]
>      [<ffffffffc181ac02>] nvmet_tcp_io_work+0x82/0x8b0 [nvmet_tcp]
>      [<ffffffff9cfa7158>] process_one_work+0x178/0x3e0
>      [<ffffffff9cfa8e9c>] worker_thread+0x2ec/0x420
>      [<ffffffff9cfb2140>] kthread+0xf0/0x120
>      [<ffffffff9cee36a4>] ret_from_fork+0x44/0x70
>      [<ffffffff9ce7fdda>] ret_from_fork_asm+0x1a/0x30
> 
> Fixes: 84909f7decbd ("nvmet: use kzalloc instead of ZERO_PAGE in
> nvme_execute_identify_ns_nvm()")
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
> ---
>   drivers/nvme/target/admin-cmd.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index e670dc185a96..acc138bbf8f2 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -1068,6 +1068,7 @@ static void nvme_execute_identify_ns_nvm(struct nvmet_req *req)
>   		goto out;
>   	}
>   	status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
> +	kfree(id);
>   out:
>   	nvmet_req_complete(req, status);
>   }
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich