linux-kernel - Re: [PATCH v3] nbd: Fix memory leak in nbd_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 22 Jun 2020 16:25:03 +0200
From:   Markus Elfring <Markus.Elfring@....de>
To:     Zheng Bin <zhengbin13@...wei.com>,
        Navid Emamdoost <navid.emamdoost@...il.com>,
        linux-block@...r.kernel.org
Cc:     nbd@...er.debian.org, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org, Aditya Pakki <pakki001@....edu>,
        Navid Emamdoost <emamd001@....edu>, Kangjie Lu <kjlu@....edu>,
        Stephen McCamant <mccamant@...umn.edu>,
        Qiushi Wu <wu000273@....edu>, Jens Axboe <axboe@...nel.dk>,
        Josef Bacik <josef@...icpanda.com>,
        Tuomas Tynkkynen <tuomas.tynkkynen@....fi>,
        Yi Zhang <yi.zhang@...wei.com>
Subject: Re: [PATCH v3] nbd: Fix memory leak in nbd_add_socket

> When adding first socket to nbd,   …

Can the term “network block device” matter for this change description?


> … A memory leak will occur
> then because the function "nbd_config_put" will free "config->socks" only
> when "config->num_connections" is not zero.

Will an additional imperative wording be helpful for the commit message?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=625d3449788f85569096780592549d0340e9c0c7#n151

Regards,
Markus


…
> +++ b/drivers/block/nbd.c
> @@ -1033,25 +1033,26 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg,
…
> +	if (!nsock) {
> +		err = -ENOMEM;
> +		goto put_socket;
>  	}
…
>  	return 0;
> +

I find that the exception handling can be improved a bit more
by adding another jump target.

+e_nomem:
+	err = -ENOMEM;


> +put_socket:
> +	sockfd_put(sock);
> +	return err;
>  }

Would you like to apply any further fine-tuning?

Regards,
Markus