2018-09-14 12:37:12

by Harry Pan

[permalink] [raw]
Subject: [PATCH] usb: core: safely deal with the dynamic quirk lists

Applying dynamic usbcore quirks in early booting when the slab is
not yet ready would cause kernel panic of null pointer dereference
because the quirk_count has been counted as 1 while the quirk_list
was failed to allocate.

i.e.,
[ 1.044970] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 1.044995] IP: [<ffffffffb0953ec7>] usb_detect_quirks+0x88/0xd1
[ 1.045016] PGD 0
[ 1.045026] Oops: 0000 [#1] PREEMPT SMP
[ 1.046986] gsmi: Log Shutdown Reason 0x03
[ 1.046995] Modules linked in:
[ 1.047008] CPU: 0 PID: 81 Comm: kworker/0:3 Not tainted 4.4.154 #28
[ 1.047016] Hardware name: Google Coral/Coral, BIOS Google_Coral.10068.27.0 12/04/2017
[ 1.047028] Workqueue: usb_hub_wq hub_event
[ 1.047037] task: ffff88017a321c80 task.stack: ffff88017a384000
[ 1.047044] RIP: 0010:[<ffffffffb0953ec7>] [<ffffffffb0953ec7>] usb_detect_quirks+0x88/0xd1

To tackle this odd, let's balance the quirk_count to 0 when the kcalloc
call fails, and defer the quirk setting into a lower level callback
which ensures that the kernel memory management has been initialized.

Fixes: 027bd6cafd9a ("usb: core: Add "quirks" parameter for usbcore")

Signed-off-by: Harry Pan <[email protected]>
---
drivers/usb/core/quirks.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 097057d2eacf..1fe9843fb9c8 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -58,6 +58,7 @@ static int quirks_param_set(const char *val, const struct kernel_param *kp)
quirk_list = kcalloc(quirk_count, sizeof(struct quirk_entry),
GFP_KERNEL);
if (!quirk_list) {
+ quirk_count = 0;
mutex_unlock(&quirk_mutex);
return -ENOMEM;
}
@@ -154,7 +155,7 @@ static struct kparam_string quirks_param_string = {
.string = quirks_param,
};

-module_param_cb(quirks, &quirks_param_ops, &quirks_param_string, 0644);
+device_param_cb(quirks, &quirks_param_ops, &quirks_param_string, 0644);
MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying quirks=vendorID:productID:quirks");

/* Lists of quirky USB devices, split in device quirks and interface quirks.
--
2.16.4



2018-09-17 07:06:32

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] usb: core: safely deal with the dynamic quirk lists

at 16:58, Harry Pan <[email protected]> wrote:

> Applying dynamic usbcore quirks in early booting when the slab is
> not yet ready would cause kernel panic of null pointer dereference
> because the quirk_count has been counted as 1 while the quirk_list
> was failed to allocate.
>
> i.e.,
> [ 1.044970] BUG: unable to handle kernel NULL pointer dereference
> at (null)
> [ 1.044995] IP: [<ffffffffb0953ec7>] usb_detect_quirks+0x88/0xd1
> [ 1.045016] PGD 0
> [ 1.045026] Oops: 0000 [#1] PREEMPT SMP
> [ 1.046986] gsmi: Log Shutdown Reason 0x03
> [ 1.046995] Modules linked in:
> [ 1.047008] CPU: 0 PID: 81 Comm: kworker/0:3 Not tainted 4.4.154 #28
> [ 1.047016] Hardware name: Google Coral/Coral, BIOS
> Google_Coral.10068.27.0 12/04/2017
> [ 1.047028] Workqueue: usb_hub_wq hub_event
> [ 1.047037] task: ffff88017a321c80 task.stack: ffff88017a384000
> [ 1.047044] RIP: 0010:[<ffffffffb0953ec7>] [<ffffffffb0953ec7>]
> usb_detect_quirks+0x88/0xd1
>
> To tackle this odd, let's balance the quirk_count to 0 when the kcalloc
> call fails, and defer the quirk setting into a lower level callback
> which ensures that the kernel memory management has been initialized.
>
> Fixes: 027bd6cafd9a ("usb: core: Add "quirks" parameter for usbcore")
>
> Signed-off-by: Harry Pan <[email protected]>

Thanks for the catch!

This issue doesn't appear when usbcore is built as module.

Acked-by: Kai-Heng Feng <[email protected]>

> ---
> drivers/usb/core/quirks.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index 097057d2eacf..1fe9843fb9c8 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -58,6 +58,7 @@ static int quirks_param_set(const char *val, const
> struct kernel_param *kp)
> quirk_list = kcalloc(quirk_count, sizeof(struct quirk_entry),
> GFP_KERNEL);
> if (!quirk_list) {
> + quirk_count = 0;
> mutex_unlock(&quirk_mutex);
> return -ENOMEM;
> }
> @@ -154,7 +155,7 @@ static struct kparam_string quirks_param_string = {
> .string = quirks_param,
> };
>
> -module_param_cb(quirks, &quirks_param_ops, &quirks_param_string, 0644);
> +device_param_cb(quirks, &quirks_param_ops, &quirks_param_string, 0644);
> MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying quirks=vendorID:productID:quirks");
>
> /* Lists of quirky USB devices, split in device quirks and interface quirks.
> --
> 2.16.4