Re: [PATCH V2 3/3] nvmet-tcp: fix memory leak when performing a controller reset
From: Maurizio Lombardi <hidden>
Date: 2021-11-16 09:23:06
On Tue, Nov 16, 2021 at 11:07:29AM +0200, Sagi Grimberg wrote:
quoted
If a reset controller is executed while the initiator is performing some I/O the driver may leak the memory allocated for the commands' iovec. Make sure that nvmet_tcp_uninit_data_in_cmds() releases all the memory. Signed-off-by: Maurizio Lombardi <redacted> --- drivers/nvme/target/tcp.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c index 028b47ff0f0f..5a238fa067cd 100644 --- a/drivers/nvme/target/tcp.c +++ b/drivers/nvme/target/tcp.c@@ -1427,7 +1427,10 @@ static void nvmet_tcp_uninit_data_in_cmds(struct nvmet_tcp_queue *queue) for (i = 0; i < queue->nr_cmds; i++, cmd++) { if (nvmet_tcp_need_data_in(cmd)) - nvmet_tcp_finish_cmd(cmd); + nvmet_req_uninit(&cmd->req); + + nvmet_tcp_unmap_pdu_iovec(cmd); + nvmet_tcp_free_iovec(cmd);I understand how unmap_pdu_iovec is re-entrance safe, but sgl_free called from free_iovec is not. What prevents it from being called for commands that have already freed their sgls?
The commands that have already freed their sgls will have the pointers set to NULL, calling sgl_free() against a NULL pointer is safe (it does nothing, like kfree()) but if you prefer I will add a check. Maurizio