2023-10-29 21:24:07

by Christophe JAILLET

[permalink] [raw]
Subject: [PATCH] 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]>
---
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..3c35c37112e6 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1429,7 +1429,8 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
if (ctype != TLS_RECORD_TYPE_DATA) {
pr_err("queue %d: unhandled TLS record %d\n",
nvme_tcp_queue_id(queue), ctype);
- return -ENOTCONN;
+ ret = -ENOTCONN;
+ goto free_icresp;
}
}
ret = -EINVAL;
--
2.34.1


2023-10-30 13:30:03

by Christoph Hellwig

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

On Sun, Oct 29, 2023 at 10:22:57PM +0100, Christophe JAILLET wrote:
> if (ctype != TLS_RECORD_TYPE_DATA) {
> pr_err("queue %d: unhandled TLS record %d\n",
> nvme_tcp_queue_id(queue), ctype);
> - return -ENOTCONN;
> + ret = -ENOTCONN;
> + goto free_icresp;
> }
> }
> ret = -EINVAL;

I'd slightly prefer the code to be consistent how it assigns to err,
and use the style where it is assigned in the main path as with the
-EINVAL for the next checks.

Except for that this looks good:

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

2023-10-31 07:10:20

by Hannes Reinecke

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

On 10/29/23 22:22, 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]>
> ---
> 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..3c35c37112e6 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -1429,7 +1429,8 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
> if (ctype != TLS_RECORD_TYPE_DATA) {
> pr_err("queue %d: unhandled TLS record %d\n",
> nvme_tcp_queue_id(queue), ctype);
> - return -ENOTCONN;
> + ret = -ENOTCONN;
> + goto free_icresp;
> }
> }
> ret = -EINVAL;

Reviewed-by: Hannes Reinecke <[email protected]>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
[email protected] +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

2023-11-20 13:51:20

by Sagi Grimberg

[permalink] [raw]