2023-10-30 14:49:53

by Christophe JAILLET

Subject: [PATCH v2] nvme-tcp: Fix a memory leak

All error handling path end to the error handling path, except this one.
Go to the error handling branch as well here, otherwise 'icreq' and
'icresp' will leak.

Fixes: 2837966ab2a8 ("nvme-tcp: control message handling for recvmsg()")
Signed-off-by: Christophe JAILLET <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
---
v2: - move ret = -xx; to the main path [Christoph Hellwig]
- Add R-b tag

v1: https://lore.kernel.org/all/f9420cde9afdc5af40bf8a8d5aa9184a9b5da729.1698614556.git.christophe.jaillet@wanadoo.fr/

Personally I prefer v1. Pick the one you prefer :)
---
drivers/nvme/host/tcp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 4714a902f4ca..f97711fc9f9f 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1423,13 +1423,14 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
nvme_tcp_queue_id(queue), ret);
goto free_icresp;
}
+ ret = -ENOTCONN;
if (queue->ctrl->ctrl.opts->tls) {
ctype = tls_get_record_type(queue->sock->sk,
(struct cmsghdr *)cbuf);
if (ctype != TLS_RECORD_TYPE_DATA) {
pr_err("queue %d: unhandled TLS record %d\n",
nvme_tcp_queue_id(queue), ctype);
- return -ENOTCONN;
+ goto free_icresp;
}
}
ret = -EINVAL;
--
2.34.1

2023-10-30 14:52:31

by Christoph Hellwig

Subject: Re: [PATCH v2] nvme-tcp: Fix a memory leak

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

> Personally I prefer v1. Pick the one you prefer :)

I think we should be consistent with one style or another in a
given function. Otherwise I wouldn't even have mentioned it.

2023-10-30 15:03:10

by Keith Busch

Subject: Re: [PATCH v2] nvme-tcp: Fix a memory leak

On Mon, Oct 30, 2023 at 03:49:28PM +0100, Christophe JAILLET wrote:
> All error handling path end to the error handling path, except this one.
> Go to the error handling branch as well here, otherwise 'icreq' and
> 'icresp' will leak.
>
> Fixes: 2837966ab2a8 ("nvme-tcp: control message handling for recvmsg()")
> Signed-off-by: Christophe JAILLET <[email protected]>
> Reviewed-by: Christoph Hellwig <[email protected]>

Thanks, applied to nvme-6.7.

2023-11-20 13:54:15

by Sagi Grimberg

Subject: Re: [PATCH v2] nvme-tcp: Fix a memory leak

> Looks good:
>
> Reviewed-by: Christoph Hellwig <[email protected]>
>
>> Personally I prefer v1. Pick the one you prefer :)
>
> I think we should be consistent with one style or another in a
> given function. Otherwise I wouldn't even have mentioned it.

This looks good too:
Reviewed-by: Sagi Grimberg <[email protected]>