linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* USB: Fix invalid-free bug in port_over_current_notify()
@ 2018-11-28 16:25 Alan Stern
  0 siblings, 0 replies; only message in thread
From: Alan Stern @ 2018-11-28 16:25 UTC (permalink / raw)
  To: Greg KH
  Cc: syzbot, drinkcat, jflat, joe, Kernel development list, USB list,
	syzkaller-bugs

Syzbot and KASAN found the following invalid-free bug in
port_over_current_notify():

--------------------------------------------------------------------------
BUG: KASAN: double-free or invalid-free in port_over_current_notify  
drivers/usb/core/hub.c:5192 [inline]
BUG: KASAN: double-free or invalid-free in port_event  
drivers/usb/core/hub.c:5241 [inline]
BUG: KASAN: double-free or invalid-free in hub_event+0xd97/0x4140  
drivers/usb/core/hub.c:5384

CPU: 1 PID: 32710 Comm: kworker/1:3 Not tainted 4.20.0-rc3+ #129
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x244/0x39d lib/dump_stack.c:113
  print_address_description.cold.7+0x9/0x1ff mm/kasan/report.c:256
  kasan_report_invalid_free+0x64/0xa0 mm/kasan/report.c:336
  __kasan_slab_free+0x13a/0x150 mm/kasan/kasan.c:501
  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
  __cache_free mm/slab.c:3498 [inline]
  kfree+0xcf/0x230 mm/slab.c:3817
  port_over_current_notify drivers/usb/core/hub.c:5192 [inline]
  port_event drivers/usb/core/hub.c:5241 [inline]
  hub_event+0xd97/0x4140 drivers/usb/core/hub.c:5384
  process_one_work+0xc90/0x1c40 kernel/workqueue.c:2153
  worker_thread+0x17f/0x1390 kernel/workqueue.c:2296
  kthread+0x35a/0x440 kernel/kthread.c:246
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
--------------------------------------------------------------------------

The problem is caused by use of a static array to store
environment-string pointers.  When the routine is called by multiple
threads concurrently, the pointers from one thread can overwrite those
from another.

The solution is to use an ordinary automatic array instead of a static
array.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: syzbot+98881958e1410ec7e53c@syzkaller.appspotmail.com
---


[as1881]


 drivers/usb/core/hub.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: usb-4.x/drivers/usb/core/hub.c
===================================================================
--- usb-4.x.orig/drivers/usb/core/hub.c
+++ usb-4.x/drivers/usb/core/hub.c
@@ -5163,7 +5163,7 @@ static void hub_port_connect_change(stru
 /* Handle notifying userspace about hub over-current events */
 static void port_over_current_notify(struct usb_port *port_dev)
 {
-	static char *envp[] = { NULL, NULL, NULL };
+	char *envp[3];
 	struct device *hub_dev;
 	char *port_dev_path;
 
@@ -5187,6 +5187,7 @@ static void port_over_current_notify(str
 	if (!envp[1])
 		goto exit;
 
+	envp[2] = NULL;
 	kobject_uevent_env(&hub_dev->kobj, KOBJ_CHANGE, envp);
 
 	kfree(envp[1]);

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2018-11-28 16:25 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-28 16:25 USB: Fix invalid-free bug in port_over_current_notify() Alan Stern

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).