* [PATCH] nbd: Fix memory leak in nbd_add_socket
@ 2020-06-12 9:04 Zheng Bin
2020-06-25 0:16 ` Chaitanya Kulkarni
0 siblings, 1 reply; 3+ messages in thread
From: Zheng Bin @ 2020-06-12 9:04 UTC (permalink / raw)
To: josef, axboe, linux-block, nbd; +Cc: yi.zhang, zhengbin13
nbd_add_socket
socks = krealloc(num_connections+1) -->if num_connections is 0, alloc 1
nsock = kzalloc -->If fail, will return
nbd_config_put
if (config->num_connections) -->0, not free
kfree(config->socks)
Thus memleak happens, this patch fixes that.
Signed-off-by: Zheng Bin <zhengbin13@huawei.com>
---
drivers/block/nbd.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 43cff01a5a67..3e7709317b17 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1037,21 +1037,22 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg,
return -EBUSY;
}
+ nsock = kzalloc(sizeof(struct nbd_sock), GFP_KERNEL);
+ if (!nsock) {
+ sockfd_put(sock);
+ return -ENOMEM;
+ }
+
socks = krealloc(config->socks, (config->num_connections + 1) *
sizeof(struct nbd_sock *), GFP_KERNEL);
if (!socks) {
sockfd_put(sock);
+ kfree(nsock);
return -ENOMEM;
}
config->socks = socks;
- nsock = kzalloc(sizeof(struct nbd_sock), GFP_KERNEL);
- if (!nsock) {
- sockfd_put(sock);
- return -ENOMEM;
- }
-
nsock->fallback_index = -1;
nsock->dead = false;
mutex_init(&nsock->tx_lock);
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] nbd: Fix memory leak in nbd_add_socket
2020-06-12 9:04 [PATCH] nbd: Fix memory leak in nbd_add_socket Zheng Bin
@ 2020-06-25 0:16 ` Chaitanya Kulkarni
2020-06-28 15:58 ` Eric Biggers
0 siblings, 1 reply; 3+ messages in thread
From: Chaitanya Kulkarni @ 2020-06-25 0:16 UTC (permalink / raw)
To: Zheng Bin, josef, axboe, linux-block, nbd; +Cc: yi.zhang
On 6/12/20 1:57 AM, Zheng Bin wrote:
> nbd_add_socket
> socks = krealloc(num_connections+1) -->if num_connections is 0, alloc 1
> nsock = kzalloc -->If fail, will return
>
> nbd_config_put
> if (config->num_connections) -->0, not free
> kfree(config->socks)
>
> Thus memleak happens, this patch fixes that.
>
> Signed-off-by: Zheng Bin<zhengbin13@huawei.com>
Not an nbd expert but wouldn't it be easier use following which matches
the + 1 in the nbd_add_socket() :-
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 01794cd2b6ca..e67c790039c9 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1209,9 +1209,9 @@ static void nbd_config_put(struct nbd_device *nbd)
device_remove_file(disk_to_dev(nbd->disk),
&pid_attr);
nbd->task_recv = NULL;
nbd_clear_sock(nbd);
- if (config->num_connections) {
+ if (config->num_connections + 1) {
int i;
- for (i = 0; i < config->num_connections; i++) {
+ for (i = 0; i < (config->num_connections + 1);
i++) {
sockfd_put(config->socks[i]->sock);
kfree(config->socks[i]);
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] nbd: Fix memory leak in nbd_add_socket
2020-06-25 0:16 ` Chaitanya Kulkarni
@ 2020-06-28 15:58 ` Eric Biggers
0 siblings, 0 replies; 3+ messages in thread
From: Eric Biggers @ 2020-06-28 15:58 UTC (permalink / raw)
To: Chaitanya Kulkarni; +Cc: Zheng Bin, josef, axboe, linux-block, nbd, yi.zhang
On Thu, Jun 25, 2020 at 12:16:33AM +0000, Chaitanya Kulkarni wrote:
> On 6/12/20 1:57 AM, Zheng Bin wrote:
> > nbd_add_socket
> > socks = krealloc(num_connections+1) -->if num_connections is 0, alloc 1
> > nsock = kzalloc -->If fail, will return
> >
> > nbd_config_put
> > if (config->num_connections) -->0, not free
> > kfree(config->socks)
> >
> > Thus memleak happens, this patch fixes that.
> >
> > Signed-off-by: Zheng Bin<zhengbin13@huawei.com>
This appears to address the syzbot report "memory leak in nbd_add_socket"
https://syzkaller.appspot.com/bug?id=08283193956ab772623e65242b3ed6d0e7c7d9ce
Can you please add:
Reported-by: syzbot+934037347002901b8d2a@syzkaller.appspotmail.com
>
> Not an nbd expert but wouldn't it be easier use following which matches
> the + 1 in the nbd_add_socket() :-
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 01794cd2b6ca..e67c790039c9 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -1209,9 +1209,9 @@ static void nbd_config_put(struct nbd_device *nbd)
> device_remove_file(disk_to_dev(nbd->disk),
> &pid_attr);
> nbd->task_recv = NULL;
> nbd_clear_sock(nbd);
> - if (config->num_connections) {
> + if (config->num_connections + 1) {
> int i;
> - for (i = 0; i < config->num_connections; i++) {
> + for (i = 0; i < (config->num_connections + 1);
> i++) {
> sockfd_put(config->socks[i]->sock);
> kfree(config->socks[i]);
> }
That looks wrong. The + 1 is just nbd_add_socket() preparing to append an entry
to the array.
Why not just check whether the pointer is NULL or not:
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 43cff01a5a67..cb8e86174edb 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1206,7 +1206,7 @@ static void nbd_config_put(struct nbd_device *nbd)
device_remove_file(disk_to_dev(nbd->disk), &pid_attr);
nbd->task_recv = NULL;
nbd_clear_sock(nbd);
- if (config->num_connections) {
+ if (config->socks) {
int i;
for (i = 0; i < config->num_connections; i++) {
sockfd_put(config->socks[i]->sock);
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-06-28 15:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-12 9:04 [PATCH] nbd: Fix memory leak in nbd_add_socket Zheng Bin
2020-06-25 0:16 ` Chaitanya Kulkarni
2020-06-28 15:58 ` Eric Biggers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).