[PATCH 4/4] nvmet-tcp: fix memory leak in nvmet_tcp_free_cmd_data_in_buffers()

[Sagi Grimberg]

> While tcp socket is being released, nvmet_tcp_release_queue_work() is
> called.
> It calls nvmet_tcp_free_cmd_data_in_buffers() to free CMD resources.
> But it may skip freeing resources due to unnecessary condition checks.
> So, the memory leak will occur.
> 
> In order to fix this problem, it removes unnecessary condition checks
> in nvmet_tcp_free_cmd_data_in_buffers().
> 
> This memory leak issue will occur in the target machine when a host
> sends reset command to a target.
> 
> Reproducer:
>     while :
>     do
>         echo 1 > /sys/class/nvme/nvme<NS>/reset_controller
>     done
> 
> unreferenced object 0xffff88814a5c6da0 (size 32):
>    comm "kworker/2:1H", pid 176, jiffies 4305953739 (age 72707.743s)
>    hex dump (first 32 bytes):
>      82 84 c8 04 00 ea ff ff 00 00 00 00 00 04 00 00  ................
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace:
>      [<ffffffffaa795cf7>] __kmalloc+0x47/0xc0
>      [<ffffffffab030a72>] sgl_alloc_order+0x82/0x3a0
>      [<ffffffffc086593c>] nvmet_tcp_map_data+0x1bc/0x570 [nvmet_tcp]
>      [<ffffffffc086a7d4>] nvmet_tcp_try_recv_pdu+0x7f4/0x1e20 [nvmet_tcp]
>      [<ffffffffc086c5a0>] nvmet_tcp_io_work+0x120/0x3272 [nvmet_tcp]
>      [<ffffffffaa1b059d>] process_one_work+0x81d/0x1450
>      [<ffffffffaa1b177c>] worker_thread+0x5ac/0xed0
>      [<ffffffffaa1c8ccf>] kthread+0x29f/0x340
>      [<ffffffffaa0034cf>] ret_from_fork+0x1f/0x30
> unreferenced object 0xffff888153f3e1c0 (size 16):
>    comm "kworker/2:1H", pid 176, jiffies 4305953739 (age 72707.743s)
>    hex dump (first 16 bytes):
>      80 84 c8 04 00 ea ff ff 00 04 00 00 00 00 00 00  ................
>    backtrace:
>      [<ffffffffaa795cf7>] __kmalloc+0x47/0xc0
>      [<ffffffffc0865a80>] nvmet_tcp_map_data+0x300/0x570 [nvmet_tcp]
>      [<ffffffffc086a7d4>] nvmet_tcp_try_recv_pdu+0x7f4/0x1e20 [nvmet_tcp]
>      [<ffffffffc086c5a0>] nvmet_tcp_io_work+0x120/0x3272 [nvmet_tcp]
>      [<ffffffffaa1b059d>] process_one_work+0x81d/0x1450
>      [<ffffffffaa1b177c>] worker_thread+0x5ac/0xed0
>      [<ffffffffaa1c8ccf>] kthread+0x29f/0x340
>      [<ffffffffaa0034cf>] ret_from_fork+0x1f/0x30
> 
> Fixes: db94f240280c ("nvmet-tcp: fix NULL pointer dereference during release")
> Signed-off-by: Taehee Yoo <ap420073 at gmail.com>
> ---
>   drivers/nvme/target/tcp.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index cc05c094de22..dac08603fec9 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -1427,13 +1427,10 @@ static void nvmet_tcp_free_cmd_data_in_buffers(struct nvmet_tcp_queue *queue)
>   	struct nvmet_tcp_cmd *cmd = queue->cmds;
>   	int i;
>   
> -	for (i = 0; i < queue->nr_cmds; i++, cmd++) {
> -		if (nvmet_tcp_need_data_in(cmd))
> -			nvmet_tcp_free_cmd_buffers(cmd);
> -	}
> +	for (i = 0; i < queue->nr_cmds; i++, cmd++)
> +		nvmet_tcp_free_cmd_buffers(cmd);
>   
> -	if (!queue->nr_cmds && nvmet_tcp_need_data_in(&queue->connect))
> -		nvmet_tcp_free_cmd_buffers(&queue->connect);
> +	nvmet_tcp_free_cmd_buffers(&queue->connect);
>   }
>   
>   static void nvmet_tcp_release_queue_work(struct work_struct *w)

Would it be possible to understand what are the commands that are
leaking here? commands that do not wait for data from the host, should
have a normal path of release, if that is not happening, we may be in
risk for use-after-free or still have a leak somewhere.