Thread (10 messages) 10 messages, 2 authors, 2021-11-16

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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help