Re: [syzbot] memory leak in usb_get_configuration

[Oliver Neukum]

On 10.03.22 00:54, syzbot wrote:

> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit:    0014404f9c18 Merge branch 'akpm' (patches from Andrew)
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=15864216700000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=3f0a704147ec8e32
> dashboard link: https://syzkaller.appspot.com/bug?extid=f0fae482604e6d9a87c9
> compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=13a63dbe700000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10e150a1700000
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+f0fae482604e6d9a87c9@xxxxxxxxxxxxxxxxxxxxxxxxx
>
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 0014404f9c18

 From 785609ab0d95c753dc31267b3c4da585c16e0274 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@xxxxxxxx>
Date: Thu, 10 Mar 2022 10:40:36 +0100
Subject: [PATCH] USB: hub: fix memory leak on failure of usb_get_config

kfree()s on the error path need to be added.

Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
---
 drivers/usb/core/config.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 48bc8a4814ac..548ce5ca6847 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -885,12 +885,16 @@ int usb_get_configuration(struct usb_device *dev)
 
 	length = ncfg * sizeof(char *);
 	dev->rawdescriptors = kzalloc(length, GFP_KERNEL);
-	if (!dev->rawdescriptors)
-		return -ENOMEM;
+	if (!dev->rawdescriptors) {
+		result = -ENOMEM;
+		goto err2;
+	}
 
 	desc = kmalloc(USB_DT_CONFIG_SIZE, GFP_KERNEL);
-	if (!desc)
-		return -ENOMEM;
+	if (!desc) {
+		result = -ENOMEM;
+		goto err2;
+	}
 
 	for (cfgno = 0; cfgno < ncfg; cfgno++) {
 		/* We grab just the first descriptor so we know how long
@@ -952,6 +956,11 @@ int usb_get_configuration(struct usb_device *dev)
 err:
 	kfree(desc);
 	dev->descriptor.bNumConfigurations = cfgno;
+err2:
+	kfree(dev->rawdescriptors);
+	kfree(dev->config);
+	dev->rawdescriptors = NULL;
+	dev->config = NULL;
 
 	return result;
 }
-- 
2.34.1