Begin forwarded message: Date: Sun, 16 Jan 2022 12:58:35 +0000 From: bugzilla-daemon@xxxxxxxxxxxxxxxxxxx To: stephen@xxxxxxxxxxxxxxxxxx Subject: [Bug 215497] New: NULL pointer deref in klist_next triggered by Bluetooth HCI_Disconnection_Complete event https://bugzilla.kernel.org/show_bug.cgi?id=215497 Bug ID: 215497 Summary: NULL pointer deref in klist_next triggered by Bluetooth HCI_Disconnection_Complete event Product: Networking Version: 2.5 Kernel Version: 5.16 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Other Assignee: stephen@xxxxxxxxxxxxxxxxxx Reporter: me@xxxxxxxxx Regression: No Created attachment 300278 --> https://bugzilla.kernel.org/attachment.cgi?id=300278&action=edit Coverage when processing the 4 frames Hi, While fuzzing bluetooth-next with KASAN I found a reproducable null-pointer-deref in klist_next. I am not sure, whether the bug lies in how the Bluetooth subsystem handles the device or in the device implementation in /drivers/base/core.c It is also triggered without KASAN and in current linux v5.16 mainline kernel, but I obtained the following logs and coverage from bluetooth-next@72279d17df54d5e4e7910b39c61a3f3464e36633. To trigger the bug, the following 4 frames must be sent by the controller (the frames sent by the fuzzer are longer, I removed the unparsed extra bytes): Frame #1: HCI Sent Connect Request (BD_ADDR 00:00:00:00:00:04) 0000 04 04 04 04 00 00 00 00 00 00 10 21 00 Frame #2: HCI_Connection_Complete (BD_ADDR 00:00:00:00:00:04, Handle 0x0010) 0000 04 03 80 00 10 00 04 00 00 00 00 00 00 00 Frame #3: HCI_Connection_Complete (duplicate of previous frame) 0000 04 03 80 00 10 00 04 00 00 00 00 00 00 00 Frame #4: HCI_Disconnection_Complete (Handle 0x0010) 0000 04 05 00 00 10 00 00 ff e8 04 94 05 ff I obtained the kcov-coverage of the bluetooth subsystem from receiving these frames from my fuzzer resolved with addr2line and attached it. This is the kernel backtrace (with KASAN): KASAN: null-ptr-deref in range [0x0000000000000058-0x000000000000005f] CPU: 0 PID: 44 Comm: kworker/u3:2 Not tainted 5.16.0-rc8+ #3 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014 Workqueue: hci0 hci_rx_work RIP: 0010:klist_next+0x44/0x450 Code: 53 48 89 fb 48 83 ec 08 80 3c 02 00 0f 85 7a 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 2b 48 8d 7d 58 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 4e 03 00 00 4c 8d 6b 08 4c 8b 7d 58 48 b8 00 00 RSP: 0018:ffffc900004ef9d8 EFLAGS: 00010212 RAX: dffffc0000000000 RBX: ffffc900004efa40 RCX: ffffffff834b4762 RDX: 000000000000000b RSI: ffffc900004efa40 RDI: 0000000000000058 RBP: 0000000000000000 R08: ffffffff844ee5a0 R09: 0000000000000001 R10: ffffffff834b4703 R11: 0000000000000000 R12: ffffffff834b43a0 R13: 1ffff9200009df44 R14: dffffc0000000000 R15: ffff88800ab18000 FS: 0000000000000000(0000) GS:ffff88806ce00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000004a26000 CR4: 00000000000006f0 Call Trace: <TASK> ? synchronize_rcu_expedited+0xb40/0xb40 ? bt_link_release+0x20/0x20 device_find_child+0xa8/0x160 ? device_for_each_child+0x140/0x140 ? mark_held_locks+0x9e/0xe0 hci_conn_del_sysfs+0xc3/0x100 hci_conn_cleanup+0x328/0x6a0 ? skb_dequeue+0x110/0x1a0 hci_conn_del+0x27e/0x6a0 ? sco_conn_del+0x2c0/0x2c0 hci_disconn_complete_evt+0x72e/0xdc0 hci_event_packet+0x7b2/0xdd0 ? hci_inquiry_complete_evt+0x560/0x560 ? hci_le_ext_adv_term_evt+0x850/0x850 ? lockdep_hardirqs_on_prepare+0x17b/0x400 hci_rx_work+0x4d3/0xb90 process_one_work+0x904/0x1590 ? lock_release+0x6f0/0x6f0 ? pwq_dec_nr_in_flight+0x230/0x230 ? rwlock_bug.part.0+0x90/0x90 ? _raw_spin_lock_irq+0x41/0x50 worker_thread+0x57c/0x1260 ? process_one_work+0x1590/0x1590 kthread+0x3b2/0x490 ? _raw_spin_unlock_irq+0x1f/0x40 ? set_kthread_struct+0x100/0x100 ret_from_fork+0x22/0x30 </TASK> Modules linked in: ---[ end trace b5a710d8df676911 ]--- RIP: 0010:klist_next+0x44/0x450 Code: 53 48 89 fb 48 83 ec 08 80 3c 02 00 0f 85 7a 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 2b 48 8d 7d 58 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 4e 03 00 00 4c 8d 6b 08 4c 8b 7d 58 48 b8 00 00 RSP: 0018:ffffc900004ef9d8 EFLAGS: 00010212 RAX: dffffc0000000000 RBX: ffffc900004efa40 RCX: ffffffff834b4762 RDX: 000000000000000b RSI: ffffc900004efa40 RDI: 0000000000000058 RBP: 0000000000000000 R08: ffffffff844ee5a0 R09: 0000000000000001 R10: ffffffff834b4703 R11: 0000000000000000 R12: ffffffff834b43a0 R13: 1ffff9200009df44 R14: dffffc0000000000 R15: ffff88800ab18000 FS: 0000000000000000(0000) GS:ffff88806ce00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000004a26000 CR4: 00000000000006f0 I tried to pin it down, the problem seems to be that conn->dev->p of the connection that should be cleaned up in hci_conn_del_sysfs is (already) NULL. In the subsequent calls, conn->dev->p->klist_children is used to initialize an iterator which is finally dereferenced in klist_next: void hci_conn_del_sysfs(struct hci_conn *conn) { struct hci_dev *hdev = conn->hdev; if (!device_is_registered(&conn->dev)) return; while (1) { struct device *dev; dev = device_find_child(&conn->dev, NULL, __match_tty); struct device *device_find_child(struct device *parent, void *data, int (*match)(struct device *dev, void *data)) { struct klist_iter i; struct device *child; if (!parent) return NULL; klist_iter_init(&parent->p->klist_children, &i); while ((child = next_device(&i))) static struct device *next_device(struct klist_iter *i) { struct klist_node *n = klist_next(i); I guess that happens during the processing of the third frame, which is a duplicate connection_complete event. During the processing of that, a warning is written to the kernel log, because sysfs tries to create a duplicate filename: debugfs: Directory '16' with parent 'hci0' already present! sysfs: cannot create duplicate filename '/devices/virtual/bluetooth/hci0/hci0:16' CPU: 0 PID: 44 Comm: kworker/u3:2 Not tainted 5.16.0-rc8+ #3 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014 Workqueue: hci0 hci_rx_work Call Trace: <TASK> dump_stack_lvl+0x45/0x59 sysfs_warn_dup.cold+0x17/0x24 sysfs_create_dir_ns+0x1eb/0x260 ? sysfs_create_mount_point+0x80/0x80 ? rwlock_bug.part.0+0x90/0x90 ? do_raw_spin_unlock+0x4f/0x210 kobject_add_internal+0x223/0x920 ? lock_release+0x3b2/0x6f0 kobject_add+0x120/0x190 ? mark_held_locks+0x9e/0xe0 ? kset_create_and_add+0x170/0x170 ? kasan_quarantine_put+0x87/0x1d0 ? lockdep_hardirqs_on+0x79/0x100 ? kasan_quarantine_put+0x87/0x1d0 ? kobject_set_name_vargs+0xad/0x110 ? kobject_get+0x50/0xe0 device_add+0x2cd/0x1b20 ? dev_set_name+0xa6/0xd0 ? device_initialize+0x540/0x540 ? __fw_devlink_link_to_suppliers+0x480/0x480 ? hci_debugfs_create_conn+0x17b/0x1f0 ? hci_debugfs_create_le+0x850/0x850 hci_conn_add_sysfs+0x8d/0xf0 hci_conn_complete_evt+0x4db/0x10b0 ? lock_chain_count+0x20/0x20 ? hci_encrypt_change_evt+0xf20/0xf20 ? lock_is_held_type+0xd7/0x130 hci_event_packet+0x7b2/0xdd0 ? hci_encrypt_change_evt+0xf20/0xf20 ? hci_le_ext_adv_term_evt+0x850/0x850 ? lockdep_hardirqs_on_prepare+0x17b/0x400 hci_rx_work+0x4d3/0xb90 process_one_work+0x904/0x1590 ? lock_release+0x6f0/0x6f0 ? pwq_dec_nr_in_flight+0x230/0x230 ? rwlock_bug.part.0+0x90/0x90 ? _raw_spin_lock_irq+0x41/0x50 worker_thread+0x57c/0x1260 ? process_one_work+0x1590/0x1590 kthread+0x3b2/0x490 ? _raw_spin_unlock_irq+0x1f/0x40 ? set_kthread_struct+0x100/0x100 ret_from_fork+0x22/0x30 </TASK> kobject_add_internal failed for hci0:16 with -EEXIST, don't try to register things with the same name in the same directory. Bluetooth: hci0: failed to register connection device I think conn->dev->p is freed in device_add in the trace that creates the previous warning, due to the third received frame, which is the second of the two duplicate HCI_Connection_Complete events: int device_add(struct device *dev) { struct device *parent; struct kobject *kobj; struct class_interface *class_intf; int error = -EINVAL; struct kobject *glue_dir = NULL; dev = get_device(dev); if (!dev) goto done; if (!dev->p) { error = device_private_init(dev); if (error) goto done; } /* * for statically allocated devices, which should all be converted * some day, we need to initialize the name. We prevent reading back * the name, and force the use of dev_name() */ if (dev->init_name) { dev_set_name(dev, "%s", dev->init_name); dev->init_name = NULL; } /* subsystems can specify simple device enumeration */ if (!dev_name(dev) && dev->bus && dev->bus->dev_name) dev_set_name(dev, "%s%u", dev->bus->dev_name, dev->id); if (!dev_name(dev)) { error = -EINVAL; goto name_error; } [..] name_error: kfree(dev->p); dev->p = NULL; goto done; } -- You may reply to this email to add a comment. You are receiving this mail because: You are the assignee for the bug.
