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

From: Tuomas Tynkkynen
Date: Fri Apr 10 2020 - 08:29:25 EST


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@xxxxxxxxxxxxxxxxxxxxxxxxx
Cc: stable@xxxxxxxxxx
Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@xxxxxx>
---
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