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

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>] [day] [month] [year] [list]

Message-Id: <20200410122913.14339-1-tuomas.tynkkynen@iki.fi>
Date:   Fri, 10 Apr 2020 15:29:13 +0300
From:   Tuomas Tynkkynen <tuomas.tynkkynen@....fi>
To:     axboe@...nel.dk, josef@...icpanda.com
Cc:     linux-block@...r.kernel.org, nbd@...er.debian.org,
        linux-kernel@...r.kernel.org,
        syzbot+934037347002901b8d2a@...kaller.appspotmail.com,
        Tuomas Tynkkynen <tuomas.tynkkynen@....fi>, stable@...nel.org
Subject: [PATCH] nbd: Fix memory leak from krealloc() if another allocation fails

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@...kaller.appspotmail.com
Cc: stable@...nel.org
Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@....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