2022-05-28 18:37:28

by Alan Stern

[permalink] [raw]
Subject: Re: [syzbot] WARNING in driver_unregister

On Sat, May 28, 2022 at 08:02:13AM -0700, syzbot wrote:
> Hello,
>
> syzbot tried to test the proposed patch but the build/boot failed:
>
> KASAN: null-ptr-deref Read in ida_free
>
> ==================================================================
> BUG: KASAN: null-ptr-deref in instrument_atomic_read include/linux/instrumented.h:71 [inline]
> BUG: KASAN: null-ptr-deref in test_bit include/asm-generic/bitops/instrumented-non-atomic.h:134 [inline]
> BUG: KASAN: null-ptr-deref in ida_free+0x1b6/0x2e0 lib/idr.c:510
> Read of size 8 at addr 0000000000000000 by task syz-fuzzer/1284
>
> CPU: 1 PID: 1284 Comm: syz-fuzzer Not tainted 5.18.0-rc5-syzkaller-00157-g97fa5887cf28-dirty #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
> print_report mm/kasan/report.c:432 [inline]
> kasan_report.cold+0x61/0x1c6 mm/kasan/report.c:491
> check_region_inline mm/kasan/generic.c:183 [inline]
> kasan_check_range+0x13d/0x180 mm/kasan/generic.c:189
> instrument_atomic_read include/linux/instrumented.h:71 [inline]
> test_bit include/asm-generic/bitops/instrumented-non-atomic.h:134 [inline]
> ida_free+0x1b6/0x2e0 lib/idr.c:510

Oops. I shouldn't try to deallocate an ID number that was never allocated
in the first place.

Alan Stern

#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 97fa5887cf28

Index: usb-devel/drivers/usb/gadget/legacy/raw_gadget.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/legacy/raw_gadget.c
+++ usb-devel/drivers/usb/gadget/legacy/raw_gadget.c
@@ -11,6 +11,7 @@
#include <linux/ctype.h>
#include <linux/debugfs.h>
#include <linux/delay.h>
+#include <linux/idr.h>
#include <linux/kref.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
@@ -36,6 +37,9 @@ MODULE_LICENSE("GPL");

/*----------------------------------------------------------------------*/

+static DEFINE_IDA(driver_id_numbers);
+#define DRIVER_DRIVER_NAME_LENGTH_MAX 32
+
#define RAW_EVENT_QUEUE_SIZE 16

struct raw_event_queue {
@@ -160,6 +164,9 @@ struct raw_dev {
/* Reference to misc device: */
struct device *dev;

+ /* Make driver names unique */
+ int driver_id_number;
+
/* Protected by lock: */
enum dev_state state;
bool gadget_registered;
@@ -188,6 +195,7 @@ static struct raw_dev *dev_new(void)
spin_lock_init(&dev->lock);
init_completion(&dev->ep0_done);
raw_event_queue_init(&dev->queue);
+ dev->driver_id_number = -1;
return dev;
}

@@ -198,6 +206,9 @@ static void dev_free(struct kref *kref)

kfree(dev->udc_name);
kfree(dev->driver.udc_name);
+ kfree(dev->driver.driver.name);
+ if (dev->driver_id_number >= 0)
+ ida_free(&driver_id_numbers, dev->driver_id_number);
if (dev->req) {
if (dev->ep0_urb_queued)
usb_ep_dequeue(dev->gadget->ep0, dev->req);
@@ -421,6 +432,7 @@ static int raw_ioctl_init(struct raw_dev
struct usb_raw_init arg;
char *udc_driver_name;
char *udc_device_name;
+ char *driver_driver_name;
unsigned long flags;

if (copy_from_user(&arg, (void __user *)value, sizeof(arg)))
@@ -439,36 +451,44 @@ static int raw_ioctl_init(struct raw_dev
return -EINVAL;
}

+ ret = ida_alloc(&driver_id_numbers, GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+ dev->driver_id_number = ret;
+
+ driver_driver_name = kmalloc(DRIVER_DRIVER_NAME_LENGTH_MAX, GFP_KERNEL);
+ if (!driver_driver_name) {
+ ret = -ENOMEM;
+ goto out_free_driver_id_number;
+ }
+ snprintf(driver_driver_name, DRIVER_DRIVER_NAME_LENGTH_MAX,
+ DRIVER_NAME ".%d", dev->driver_id_number);
+
udc_driver_name = kmalloc(UDC_NAME_LENGTH_MAX, GFP_KERNEL);
- if (!udc_driver_name)
- return -ENOMEM;
+ if (!udc_driver_name) {
+ ret = -ENOMEM;
+ goto out_free_driver_driver_name;
+ }
ret = strscpy(udc_driver_name, &arg.driver_name[0],
UDC_NAME_LENGTH_MAX);
- if (ret < 0) {
- kfree(udc_driver_name);
- return ret;
- }
+ if (ret < 0)
+ goto out_free_udc_driver_name;
ret = 0;

udc_device_name = kmalloc(UDC_NAME_LENGTH_MAX, GFP_KERNEL);
if (!udc_device_name) {
- kfree(udc_driver_name);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out_free_udc_driver_name;
}
ret = strscpy(udc_device_name, &arg.device_name[0],
UDC_NAME_LENGTH_MAX);
- if (ret < 0) {
- kfree(udc_driver_name);
- kfree(udc_device_name);
- return ret;
- }
+ if (ret < 0)
+ goto out_free_udc_device_name;
ret = 0;

spin_lock_irqsave(&dev->lock, flags);
if (dev->state != STATE_DEV_OPENED) {
dev_dbg(dev->dev, "fail, device is not opened\n");
- kfree(udc_driver_name);
- kfree(udc_device_name);
ret = -EINVAL;
goto out_unlock;
}
@@ -483,14 +503,24 @@ static int raw_ioctl_init(struct raw_dev
dev->driver.suspend = gadget_suspend;
dev->driver.resume = gadget_resume;
dev->driver.reset = gadget_reset;
- dev->driver.driver.name = DRIVER_NAME;
+ dev->driver.driver.name = driver_driver_name;
dev->driver.udc_name = udc_device_name;
dev->driver.match_existing_only = 1;

dev->state = STATE_DEV_INITIALIZED;
+ spin_unlock_irqrestore(&dev->lock, flags);
+ return ret;

out_unlock:
spin_unlock_irqrestore(&dev->lock, flags);
+out_free_udc_device_name:
+ kfree(udc_device_name);
+out_free_udc_driver_name:
+ kfree(udc_driver_name);
+out_free_driver_driver_name:
+ kfree(driver_driver_name);
+out_free_driver_id_number:
+ ida_free(&driver_id_numbers, dev->driver_id_number);
return ret;
}



2022-05-28 20:12:06

by syzbot

[permalink] [raw]
Subject: Re: [syzbot] WARNING in driver_unregister

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
WARNING in driver_unregister

------------[ cut here ]------------
Unexpected driver unregister!
WARNING: CPU: 0 PID: 2335 at drivers/base/driver.c:194 driver_unregister drivers/base/driver.c:194 [inline]
WARNING: CPU: 0 PID: 2335 at drivers/base/driver.c:194 driver_unregister+0x8c/0xb0 drivers/base/driver.c:191
Modules linked in:
CPU: 0 PID: 2335 Comm: syz-executor.0 Not tainted 5.18.0-rc5-syzkaller-00157-g97fa5887cf28-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:driver_unregister drivers/base/driver.c:194 [inline]
RIP: 0010:driver_unregister+0x8c/0xb0 drivers/base/driver.c:191
Code: 68 4c 89 e7 e8 65 b9 db fe 48 89 ef e8 fd a0 ff ff 5d 41 5c e9 75 fa 78 fe e8 70 fa 78 fe 48 c7 c7 80 7a 81 86 e8 12 96 ee 02 <0f> 0b 5d 41 5c e9 5a fa 78 fe e8 75 93 ad fe eb 96 e8 6e 93 ad fe
RSP: 0018:ffffc9000267fa78 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff888118006050 RCX: 0000000000000000
RDX: ffff888114fe8000 RSI: ffffffff812bdce8 RDI: fffff520004cff41
RBP: ffff888118006098 R08: 0000000000000000 R09: 0000000000000001
R10: ffffffff812b86be R11: 0000000000000000 R12: 0000000000000000
R13: ffff888118006008 R14: ffff88811785e7a8 R15: ffff888100219ca0
FS: 0000000000000000(0000) GS:ffff8881f6800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fc6f40b3718 CR3: 0000000007825000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
usb_gadget_unregister_driver+0x48/0x70 drivers/usb/gadget/udc/core.c:1590
raw_release+0x18b/0x290 drivers/usb/gadget/legacy/raw_gadget.c:412
__fput+0x277/0x9d0 fs/file_table.c:317
task_work_run+0xdd/0x1a0 kernel/task_work.c:164
exit_task_work include/linux/task_work.h:37 [inline]
do_exit+0xaff/0x2980 kernel/exit.c:795
do_group_exit+0xd2/0x2f0 kernel/exit.c:925
get_signal+0x22df/0x24c0 kernel/signal.c:2864
arch_do_signal_or_restart+0x82/0x20f0 arch/x86/kernel/signal.c:867
exit_to_user_mode_loop kernel/entry/common.c:166 [inline]
exit_to_user_mode_prepare+0x156/0x200 kernel/entry/common.c:201
__syscall_exit_to_user_mode_work kernel/entry/common.c:283 [inline]
syscall_exit_to_user_mode+0x19/0x60 kernel/entry/common.c:294
do_syscall_64+0x42/0xb0 arch/x86/entry/common.c:86
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7fc6f417f0e9
Code: Unable to access opcode bytes at RIP 0x7fc6f417f0bf.
RSP: 002b:00007fc6f40b3218 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
RAX: fffffffffffffe00 RBX: 00007fc6f4292108 RCX: 00007fc6f417f0e9
RDX: 0000000000000000 RSI: 0000000000000080 RDI: 00007fc6f4292108
RBP: 00007fc6f4292100 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fc6f429210c
R13: 00007ffd64cd846f R14: 00007fc6f40b3300 R15: 0000000000022000
</TASK>


Tested on:

commit: 97fa5887 USB: new quirk for Dell Gen 2 devices
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
console output: https://syzkaller.appspot.com/x/log.txt?x=16effa13f00000
kernel config: https://syzkaller.appspot.com/x/.config?x=d7b232ec3adf5c8d
dashboard link: https://syzkaller.appspot.com/bug?extid=02b16343704b3af1667e
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
patch: https://syzkaller.appspot.com/x/patch.diff?x=13e750ddf00000