[PATCH] nbd: Fix memory leak from krealloc() if another allocation fails

[PATCH] nbd: Fix memory leak from krealloc() if another allocation fails

[Tuomas Tynkkynen]

syzkaller reports a memory leak when injecting allocation failures:

FAULT_INJECTION: forcing a failure.
name failslab, interval 1, probability 0, space 0, times 0
...
 kmem_cache_alloc_trace+0x26/0x2c0
 nbd_add_socket+0xa8/0x1e0
 nbd_ioctl+0x175/0x430
...
BUG: memory leak
    [<0000000090cb73c8>] __do_krealloc mm/slab_common.c:1671 [inline]
    [<0000000090cb73c8>] krealloc+0x7c/0xa0 mm/slab_common.c:1700
    [<00000000cf9e6ba7>] nbd_add_socket+0x7d/0x1e0 drivers/block/nbd.c:1040
    ...

This happens when krealloc() succeeds but the kzalloc() fails:
1040         socks = krealloc(config->socks, (config->num_connections + 1) *
1041                          sizeof(struct nbd_sock *), GFP_KERNEL);
1042         if (!socks) {
1043                 sockfd_put(sock);
1044                 return -ENOMEM;
1045         }
1046
1047         config->socks = socks;
1048
1049         nsock = kzalloc(sizeof(struct nbd_sock), GFP_KERNEL);
1050         if (!nsock) {
1051                 sockfd_put(sock);
1052                 return -ENOMEM;
1053         }

as then config->num_connections is not incremented and the cleanup code
freeing config->socks is skipped. Just make it run always.

Reported-by: syzbot+934037347002901b8d2a@syzkaller.appspotmail.com
Cc: stable@kernel.org
Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>
---
Compile tested only.
---
 drivers/block/nbd.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 43cff01a5a67..f851883ef9f4 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1199,6 +1199,8 @@ static void nbd_config_put(struct nbd_device *nbd)
 	if (refcount_dec_and_mutex_lock(&nbd->config_refs,
 					&nbd->config_lock)) {
 		struct nbd_config *config = nbd->config;
+		int i;
+
 		nbd_dev_dbg_close(nbd);
 		nbd_size_clear(nbd);
 		if (test_and_clear_bit(NBD_RT_HAS_PID_FILE,
@@ -1206,14 +1208,11 @@ 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) {
-			int i;
-			for (i = 0; i < config->num_connections; i++) {
-				sockfd_put(config->socks[i]->sock);
-				kfree(config->socks[i]);
-			}
-			kfree(config->socks);
+		for (i = 0; i < config->num_connections; i++) {
+			sockfd_put(config->socks[i]->sock);
+			kfree(config->socks[i]);
 		}
+		kfree(config->socks);
 		kfree(nbd->config);
 		nbd->config = NULL;
 
-- 
2.17.1

Re: [PATCH] nbd: Fix memory leak from krealloc() if another allocation fails

[Markus Elfring]

> syzkaller reports a memory leak when injecting allocation failures:
…
> as then config->num_connections is not incremented and the cleanup code
> freeing config->socks is skipped. Just make it run always.

How do you think about to add the tag “Fixes” for the final change description?

Regards,
Markus

Re: [PATCH] nbd: Fix memory leak from krealloc() if another allocation fails

[Sasha Levin]

Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: .+

The bot has tested the following trees: v5.6.5, v5.5.18, v5.4.33, v4.19.116, v4.14.176, v4.9.219, v4.4.219.

v5.6.5: Build OK!
v5.5.18: Build OK!
v5.4.33: Build OK!
v4.19.116: Build OK!
v4.14.176: Build OK!
v4.9.219: Failed to apply! Possible dependencies:
    20032ec38d16 ("nbd: reset the setup task for NBD_CLEAR_SOCK")
    5ea8d10802ec ("nbd: separate out the config information")
    9442b739207a ("nbd: cleanup ioctl handling")
    9561a7ade0c2 ("nbd: add multi-connection support")
    feffa5cc7b47 ("nbd: fix setting of 'error' in NBD_DO_IT ioctl")

v4.4.219: Failed to apply! Possible dependencies:
    0e4f0f6f63d3 ("nbd: Cleanup reset of nbd and bdev after a disconnect")
    1f7b5cf1be43 ("nbd: Timeouts are not user requested disconnects")
    23272a6754b8 ("nbd: Remove signal usage")
    37091fdd831f ("nbd: Create size change events for userspace")
    5ea8d10802ec ("nbd: separate out the config information")
    9561a7ade0c2 ("nbd: add multi-connection support")
    97240963eb30 ("nbd: fix race in ioctl")
    9b4a6ba9185a ("nbd: use flags instead of bool")
    fd8383fd88a2 ("nbd: convert to blkmq")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha