commit 7feca65b6481514ffadcd64905612d91d23fcd39
Author: Mathias Payer <mathias.payer@nebelwelt.net>
Date: Tue Dec 18 11:12:28 2018 +0100
Fix NULL deref in drivers/net/wireless/ath/ath{6kl|10k}/usb.c
ath{6kl|10k}_usb_alloc_urb_from_pipe does not check if the pipe
is valid and that is has been correctly allocated. Add a NULL
check before accessing the pipe.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
b/drivers/net/wireless/ath/ath10k/usb.c
@@ -49,6 +49,11 @@ ath10k_usb_alloc_urb_from_pipe(struct ath10k_usb_pipe *pipe)
struct ath10k_urb_context *urb_context = NULL;
unsigned long flags;
+ /* bail if this pipe is not allocated */
+ if (pipe->ar_usb == NULL) {
+ return NULL;
+ }
+
spin_lock_irqsave(&pipe->ar_usb->cs_lock, flags);
if (!list_empty(&pipe->urb_list_head)) {
urb_context = list_first_entry(&pipe->urb_list_head,
b/drivers/net/wireless/ath/ath6kl/usb.c
@@ -132,6 +132,11 @@ ath6kl_usb_alloc_urb_from_pipe(struct ath6kl_usb_pipe *pipe)
struct ath6kl_urb_context *urb_context = NULL;
unsigned long flags;
+ /* bail if this pipe is not allocated */
+ if (pipe->ar_usb == NULL) {
+ return NULL;
+ }
+
spin_lock_irqsave(&pipe->ar_usb->cs_lock, flags);
if (!list_empty(&pipe->urb_list_head)) {
urb_context =
Hi there, We have developed a USB testing framework and found two NULL derefes in the ath driver when an attacker-controlled USB device sends a malformed USB packet. The bug is in drivers/net/wireless/ath/ath{6kl|10k}/usb.c We show the bug for ath6kl/usb.c but the code seems to be copied over to ath10k/usb.c and it is equally vulnerable. ``` ath6kl_usb_alloc_urb_from_pipe(struct ath6kl_usb_pipe *pipe) { struct ath6kl_urb_context *urb_context = NULL; unsigned long flags; // buggy statement spin_lock_irqsave(&pipe->ar_usb->cs_lock, flags); if (!list_empty(&pipe->urb_list_head)) { urb_context = list_first_entry(&pipe->urb_list_head, struct ath6kl_urb_context, link); list_del(&urb_context->link); pipe->urb_cnt--; } spin_unlock_irqrestore(&pipe->ar_usb->cs_lock, flags); return urb_context; } ``` The `field pipe->ar_usb` may be NULL if the corresponding pipes have not been setup, resulting in a NULL pointer deref, when the spin_lock tries to access the `cs_lock` field. During setup, the initialization code allocates the pipes according to whatever is requested from the USB channel, potentially missing the initialization of some pipes. # Code flow 1. `ath6kl_usb_probe` calls ` ath6kl_usb_create` to create an `ath6kl_usb` object 2. Inside `ath6kl_usb_create` (https://elixir.bootlin.com/linux/v4.14.81/source/drivers/net/wireless/ath/ath6kl/usb.c#L613) a. it allocates memory for the ath6kl_usb object using `kzalloc` b. the array of `ath6kl_usb_pipe` (`pipes` field) is setup by calling `ath6kl_usb_setup_pipe_resources`, where it may goes wrong 3. The `ath6kl_usb_setup_pipe_resources` (https://elixir.bootlin.com/linux/v4.14.81/source/drivers/net/wireless/ath/ath6kl/usb.c#L295) function iterates through all endpoints reported by the device and initializes each pipe according to the endpoint information It calculates the pipnum (the index to the pipes array) according to the endpoint address by calling `ath6kl_usb_get_logical_pipe_num`, and then sets up the pipe object. The selected pipe object's `ar_usb` field will be set to the containing `ath6kl_usb` object. If a pipe object is not selected by the endpoint information, their `ar_usb` field will be zero, as the underlying `ath6kl_usb` object is allocated by `kzalloc`. The driver assumes that all endpoint addresses are complete (covering range 0 to MAX_PIPE_NUM), assuming all pipe objects are setup correctly. A malicious USB device can force some pipes to not be setup. # Proposed fix As a simple fix, we propose to check for NULL in ath6kl_usb_alloc_urb_from_pipe, making sure the underlying object was correctly initialized. This *should* work for the two drivers and will be a quick fix. The cleanest solution would check at the end of `ath6kl_usb_setup_pipe_resources` that all required pipes are set up correctly. The question is what is the minimum set of pipes that the hardware exports? As I don't have access to the real hardware, I cannot test this on a valid device. Looking through the code, I see that the the driver itself expects at least pipes[ATH6KL_USB_PIPE_RX_DATA] #482, #484, #1188 pipes[ATH6KL_USB_PIPE_RX_DATA2] #1190 to be valid as they are accessed directly. `hif_start` #677 seems to expect that all TX pipes are setup: ``` for (i = ATH6KL_USB_PIPE_TX_CTRL; i <= ATH6KL_USB_PIPE_TX_DATA_HP; i++) { device->pipes[i].urb_cnt_thresh = device->pipes[i].urb_alloc / 2; } ``` -> ATH6KL_USB_PIPE_TX_CTRL = 0, ATH6KL_USB_PIPE_TX_DATA_LP, ATH6KL_USB_PIPE_TX_DATA_MP, ATH6KL_USB_PIPE_TX_DATA_HP, But, there are other accesses to pipes[XXX] where XXX is passed into the module from outside. I don't know the network stack well enough to infer what values are passed in. If we can assume that all pipes in `enum ATH6KL_USB_PIPE_ID` #30 are setup, then we could simply iterate through the pipe array at the end of `ath6kl_usb_setup_pipe_resources` and check that they are all non-NULL. The simple (attached) fix is likely enough but someone with access to the hardware may want to develop a more extensive fix. Let me know what you think :) Best, Mathias and Hui BUG: KASAN: null-ptr-deref in __lock_acquire+0x5a8/0x1b40 kernel/locking/lockdep.c:3365 CPU: 0 PID: 5496 Comm: test Not tainted 4.14.81 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014 Call Trace: __dump_stack lib/dump_stack.c:17 [inline] dump_stack+0xc1/0x127 lib/dump_stack.c:53 kasan_report_error mm/kasan/report.c:349 [inline] kasan_report.cold.6+0x6d/0x2d3 mm/kasan/report.c:409 check_memory_region_inline mm/kasan/kasan.c:260 [inline] __asan_load8+0x54/0x90 mm/kasan/kasan.c:694 __lock_acquire+0x5a8/0x1b40 kernel/locking/lockdep.c:3365 lock_acquire+0xc9/0x200 kernel/locking/lockdep.c:3991 __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] _raw_spin_lock_irqsave+0x4a/0x5e kernel/locking/spinlock.c:160 ath6kl_usb_alloc_urb_from_pipe+0x3c/0x130 drivers/net/wireless/ath/ath6kl/usb.c:135 ath6kl_usb_post_recv_transfers.constprop.10+0x17e/0x210 drivers/net/wireless/ath/ath6kl/usb.c:410 ath6kl_usb_start_recv_pipes drivers/net/wireless/ath/ath6kl/usb.c:484 [inline] hif_start drivers/net/wireless/ath/ath6kl/usb.c:682 [inline] ath6kl_usb_power_on+0x4b/0xa0 drivers/net/wireless/ath/ath6kl/usb.c:1041 ath6kl_hif_power_on drivers/net/wireless/ath/ath6kl/hif-ops.h:136 [inline] ath6kl_core_init+0x119/0x7c0 drivers/net/wireless/ath/ath6kl/core.c:97 ath6kl_usb_probe+0x748/0x8e0 drivers/net/wireless/ath/ath6kl/usb.c:1147 usb_probe_interface+0x1b5/0x450 drivers/usb/core/driver.c:361 really_probe drivers/base/dd.c:405 [inline] driver_probe_device+0x381/0x460 drivers/base/dd.c:549 __device_attach_driver+0x13a/0x160 drivers/base/dd.c:645 bus_for_each_drv+0xdb/0x130 drivers/base/bus.c:463 __device_attach+0x14b/0x1d0 drivers/base/dd.c:702 device_initial_probe+0x1f/0x30 drivers/base/dd.c:749 bus_probe_device+0x14a/0x160 drivers/base/bus.c:523 device_add+0x570/0xaf0 drivers/base/core.c:1849 usb_set_configuration+0x875/0xcc0 drivers/usb/core/message.c:1947 generic_probe+0x7d/0xb0 drivers/usb/core/generic.c:174 usb_probe_device+0x64/0xa0 drivers/usb/core/driver.c:266 really_probe drivers/base/dd.c:405 [inline] driver_probe_device+0x381/0x460 drivers/base/dd.c:549 __device_attach_driver+0x13a/0x160 drivers/base/dd.c:645 bus_for_each_drv+0xdb/0x130 drivers/base/bus.c:463 __device_attach+0x14b/0x1d0 drivers/base/dd.c:702 device_initial_probe+0x1f/0x30 drivers/base/dd.c:749 bus_probe_device+0x14a/0x160 drivers/base/bus.c:523 device_add+0x570/0xaf0 drivers/base/core.c:1849 usb_new_device+0x584/0xd60 drivers/usb/core/hub.c:2544 hub_port_connect drivers/usb/core/hub.c:5002 [inline] hub_port_connect_change drivers/usb/core/hub.c:5117 [inline] port_event drivers/usb/core/hub.c:5223 [inline] hub_event_impl+0x10be/0x1f80 drivers/usb/core/hub.c:5335 hub_event+0x38/0x50 drivers/usb/core/hub.c:5222 process_one_work+0x944/0x15f0 kernel/workqueue.c:2112 worker_thread+0xef/0x10d0 kernel/workqueue.c:2246 kthread+0x367/0x420 kernel/kthread.c:238 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:437 RIP: 0033:0x452d07 RSP: 002b:00007ffcbb2ff968 EFLAGS: 00000202 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000452d07 RDX: 00007ffcbb2ff980 RSI: 00000000c0105512 RDI: 0000000000000000 RBP: 00000000200f0000 R08: 000000002096cff8 R09: 000000002071ffa2 R10: 0000000000000004 R11: 0000000000000202 R12: 0000000000000024 R13: 000000002096cff8 R14: 0000000000000004 R15: 0000000000000003 ==================================================================