* [PATCH] gpio / ACPI: Don't crash on NULL chip->dev
@ 2014-03-31 12:16 Mika Westerberg
2014-03-31 16:25 ` Sabrina Dubroca
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Mika Westerberg @ 2014-03-31 12:16 UTC (permalink / raw)
To: Linus Walleij
Cc: Alexandre Courbot, Sabrina Dubroca, linux-gpio, linux-kernel,
Mika Westerberg
Commit aa92b6f689ac (gpio / ACPI: Allocate ACPI specific data directly in
acpi_gpiochip_add()) moved ACPI handle checking to acpi_gpiochip_add() but
forgot to check whether chip->dev is NULL before dereferencing it.
Since chip->dev pointer is optional we can end up with crash like following:
BUG: unable to handle kernel NULL pointer dereference at 00000138
IP: [<c126c2b3>] acpi_gpiochip_add+0x13/0x190
*pde = 00000000
Oops: 0000 [#1] PREEMPT SMP
Modules linked in: ssb(+) ...
CPU: 0 PID: 512 Comm: modprobe Tainted: G W 3.14.0-rc7-next-20140324-t1 #24
Hardware name: Dell Inc. Latitude D830 /0UY141, BIOS A02 06/07/2007
task: f5799900 ti: f543e000 task.ti: f543e000
EIP: 0060:[<c126c2b3>] EFLAGS: 00010282 CPU: 0
EIP is at acpi_gpiochip_add+0x13/0x190
EAX: 00000000 EBX: f57824c4 ECX: 00000000 EDX: 00000000
ESI: f57824c4 EDI: 00000010 EBP: f543fc54 ESP: f543fc40
DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
CR0: 8005003b CR2: 00000138 CR3: 355f8000 CR4: 000007d0
Stack:
f543fc5c fd1f7790 f57824c4 000000be 00000010 f543fc84 c1269f4e f543fc74
fd1f78bd 00008002 f57822b0 f5782090 fd1f8400 00000286 fd1f9994 00000000
f5782000 f543fc8c fd1f7e39 f543fcc8 fd1f0bd8 000000c0 00000000 00000000
Call Trace:
[<fd1f7790>] ? ssb_pcie_mdio_write+0xa0/0xd0 [ssb]
[<c1269f4e>] gpiochip_add+0xee/0x300
[<fd1f78bd>] ? ssb_pcicore_serdes_workaround+0xfd/0x140 [ssb]
[<fd1f7e39>] ssb_gpio_init+0x89/0xa0 [ssb]
[<fd1f0bd8>] ssb_attach_queued_buses+0xc8/0x2d0 [ssb]
[<fd1f0f65>] ssb_bus_register+0x185/0x1f0 [ssb]
[<fd1f3120>] ? ssb_pci_xtal+0x220/0x220 [ssb]
[<fd1f106c>] ssb_bus_pcibus_register+0x2c/0x80 [ssb]
[<fd1f40dc>] ssb_pcihost_probe+0x9c/0x110 [ssb]
[<c1276c8f>] pci_device_probe+0x6f/0xc0
[<c11bdb55>] ? sysfs_create_link+0x25/0x40
[<c131d8b9>] driver_probe_device+0x79/0x360
[<c1276512>] ? pci_match_device+0xb2/0xc0
[<c131dc51>] __driver_attach+0x71/0x80
[<c131dbe0>] ? __device_attach+0x40/0x40
[<c131bd87>] bus_for_each_dev+0x47/0x80
[<c131d3ae>] driver_attach+0x1e/0x20
[<c131dbe0>] ? __device_attach+0x40/0x40
[<c131d007>] bus_add_driver+0x157/0x230
[<c131e219>] driver_register+0x59/0xe0
...
Fix this by checking chip->dev pointer against NULL first. Also we can now
remove redundant check in acpi_gpiochip_request/free_interrupts().
Reported-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Sabrina,
Can you please re-test this and provide your tested-by? I changed the patch
a bit to remove redundant checks. Just to be sure that I don't accidentally
break something.
Thanks.
drivers/gpio/gpiolib-acpi.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index bf0f8b476696..d5be56fe689e 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -233,7 +233,7 @@ static void acpi_gpiochip_request_interrupts(struct acpi_gpio_chip *acpi_gpio)
{
struct gpio_chip *chip = acpi_gpio->chip;
- if (!chip->dev || !chip->to_irq)
+ if (!chip->to_irq)
return;
INIT_LIST_HEAD(&acpi_gpio->events);
@@ -253,7 +253,7 @@ static void acpi_gpiochip_free_interrupts(struct acpi_gpio_chip *acpi_gpio)
struct acpi_gpio_event *event, *ep;
struct gpio_chip *chip = acpi_gpio->chip;
- if (!chip->dev || !chip->to_irq)
+ if (!chip->to_irq)
return;
list_for_each_entry_safe_reverse(event, ep, &acpi_gpio->events, node) {
@@ -501,6 +501,9 @@ void acpi_gpiochip_add(struct gpio_chip *chip)
acpi_handle handle;
acpi_status status;
+ if (!chip || !chip->dev)
+ return;
+
handle = ACPI_HANDLE(chip->dev);
if (!handle)
return;
@@ -531,6 +534,9 @@ void acpi_gpiochip_remove(struct gpio_chip *chip)
acpi_handle handle;
acpi_status status;
+ if (!chip || !chip->dev)
+ return;
+
handle = ACPI_HANDLE(chip->dev);
if (!handle)
return;
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio / ACPI: Don't crash on NULL chip->dev
2014-03-31 12:16 [PATCH] gpio / ACPI: Don't crash on NULL chip->dev Mika Westerberg
@ 2014-03-31 16:25 ` Sabrina Dubroca
2014-03-31 19:33 ` Alexandre Courbot
2014-04-10 16:06 ` Linus Walleij
2 siblings, 0 replies; 4+ messages in thread
From: Sabrina Dubroca @ 2014-03-31 16:25 UTC (permalink / raw)
To: Mika Westerberg
Cc: Linus Walleij, Alexandre Courbot, linux-gpio, linux-kernel
2014-03-31, 15:16:49 +0300, Mika Westerberg wrote:
> Commit aa92b6f689ac (gpio / ACPI: Allocate ACPI specific data directly in
> acpi_gpiochip_add()) moved ACPI handle checking to acpi_gpiochip_add() but
> forgot to check whether chip->dev is NULL before dereferencing it.
>
> Since chip->dev pointer is optional we can end up with crash like following:
>
> BUG: unable to handle kernel NULL pointer dereference at 00000138
> IP: [<c126c2b3>] acpi_gpiochip_add+0x13/0x190
> *pde = 00000000
> Oops: 0000 [#1] PREEMPT SMP
> Modules linked in: ssb(+) ...
> CPU: 0 PID: 512 Comm: modprobe Tainted: G W 3.14.0-rc7-next-20140324-t1 #24
> Hardware name: Dell Inc. Latitude D830 /0UY141, BIOS A02 06/07/2007
> task: f5799900 ti: f543e000 task.ti: f543e000
> EIP: 0060:[<c126c2b3>] EFLAGS: 00010282 CPU: 0
> EIP is at acpi_gpiochip_add+0x13/0x190
> EAX: 00000000 EBX: f57824c4 ECX: 00000000 EDX: 00000000
> ESI: f57824c4 EDI: 00000010 EBP: f543fc54 ESP: f543fc40
> DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> CR0: 8005003b CR2: 00000138 CR3: 355f8000 CR4: 000007d0
> Stack:
> f543fc5c fd1f7790 f57824c4 000000be 00000010 f543fc84 c1269f4e f543fc74
> fd1f78bd 00008002 f57822b0 f5782090 fd1f8400 00000286 fd1f9994 00000000
> f5782000 f543fc8c fd1f7e39 f543fcc8 fd1f0bd8 000000c0 00000000 00000000
> Call Trace:
> [<fd1f7790>] ? ssb_pcie_mdio_write+0xa0/0xd0 [ssb]
> [<c1269f4e>] gpiochip_add+0xee/0x300
> [<fd1f78bd>] ? ssb_pcicore_serdes_workaround+0xfd/0x140 [ssb]
> [<fd1f7e39>] ssb_gpio_init+0x89/0xa0 [ssb]
> [<fd1f0bd8>] ssb_attach_queued_buses+0xc8/0x2d0 [ssb]
> [<fd1f0f65>] ssb_bus_register+0x185/0x1f0 [ssb]
> [<fd1f3120>] ? ssb_pci_xtal+0x220/0x220 [ssb]
> [<fd1f106c>] ssb_bus_pcibus_register+0x2c/0x80 [ssb]
> [<fd1f40dc>] ssb_pcihost_probe+0x9c/0x110 [ssb]
> [<c1276c8f>] pci_device_probe+0x6f/0xc0
> [<c11bdb55>] ? sysfs_create_link+0x25/0x40
> [<c131d8b9>] driver_probe_device+0x79/0x360
> [<c1276512>] ? pci_match_device+0xb2/0xc0
> [<c131dc51>] __driver_attach+0x71/0x80
> [<c131dbe0>] ? __device_attach+0x40/0x40
> [<c131bd87>] bus_for_each_dev+0x47/0x80
> [<c131d3ae>] driver_attach+0x1e/0x20
> [<c131dbe0>] ? __device_attach+0x40/0x40
> [<c131d007>] bus_add_driver+0x157/0x230
> [<c131e219>] driver_register+0x59/0xe0
> ...
>
> Fix this by checking chip->dev pointer against NULL first. Also we can now
> remove redundant check in acpi_gpiochip_request/free_interrupts().
>
> Reported-by: Sabrina Dubroca <sd@queasysnail.net>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> Sabrina,
>
> Can you please re-test this and provide your tested-by? I changed the patch
> a bit to remove redundant checks. Just to be sure that I don't accidentally
> break something.
>
> Thanks.
Everything looks good.
Tested-by: Sabrina Dubroca <sd@queasysnail.net>
Thanks,
--
Sabrina
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio / ACPI: Don't crash on NULL chip->dev
2014-03-31 12:16 [PATCH] gpio / ACPI: Don't crash on NULL chip->dev Mika Westerberg
2014-03-31 16:25 ` Sabrina Dubroca
@ 2014-03-31 19:33 ` Alexandre Courbot
2014-04-10 16:06 ` Linus Walleij
2 siblings, 0 replies; 4+ messages in thread
From: Alexandre Courbot @ 2014-03-31 19:33 UTC (permalink / raw)
To: Mika Westerberg
Cc: Linus Walleij, Sabrina Dubroca, linux-gpio@vger.kernel.org,
Linux Kernel Mailing List
On Mon, Mar 31, 2014 at 9:16 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> Commit aa92b6f689ac (gpio / ACPI: Allocate ACPI specific data directly in
> acpi_gpiochip_add()) moved ACPI handle checking to acpi_gpiochip_add() but
> forgot to check whether chip->dev is NULL before dereferencing it.
>
> Since chip->dev pointer is optional we can end up with crash like following:
>
> BUG: unable to handle kernel NULL pointer dereference at 00000138
> IP: [<c126c2b3>] acpi_gpiochip_add+0x13/0x190
> *pde = 00000000
> Oops: 0000 [#1] PREEMPT SMP
> Modules linked in: ssb(+) ...
> CPU: 0 PID: 512 Comm: modprobe Tainted: G W 3.14.0-rc7-next-20140324-t1 #24
> Hardware name: Dell Inc. Latitude D830 /0UY141, BIOS A02 06/07/2007
> task: f5799900 ti: f543e000 task.ti: f543e000
> EIP: 0060:[<c126c2b3>] EFLAGS: 00010282 CPU: 0
> EIP is at acpi_gpiochip_add+0x13/0x190
> EAX: 00000000 EBX: f57824c4 ECX: 00000000 EDX: 00000000
> ESI: f57824c4 EDI: 00000010 EBP: f543fc54 ESP: f543fc40
> DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> CR0: 8005003b CR2: 00000138 CR3: 355f8000 CR4: 000007d0
> Stack:
> f543fc5c fd1f7790 f57824c4 000000be 00000010 f543fc84 c1269f4e f543fc74
> fd1f78bd 00008002 f57822b0 f5782090 fd1f8400 00000286 fd1f9994 00000000
> f5782000 f543fc8c fd1f7e39 f543fcc8 fd1f0bd8 000000c0 00000000 00000000
> Call Trace:
> [<fd1f7790>] ? ssb_pcie_mdio_write+0xa0/0xd0 [ssb]
> [<c1269f4e>] gpiochip_add+0xee/0x300
> [<fd1f78bd>] ? ssb_pcicore_serdes_workaround+0xfd/0x140 [ssb]
> [<fd1f7e39>] ssb_gpio_init+0x89/0xa0 [ssb]
> [<fd1f0bd8>] ssb_attach_queued_buses+0xc8/0x2d0 [ssb]
> [<fd1f0f65>] ssb_bus_register+0x185/0x1f0 [ssb]
> [<fd1f3120>] ? ssb_pci_xtal+0x220/0x220 [ssb]
> [<fd1f106c>] ssb_bus_pcibus_register+0x2c/0x80 [ssb]
> [<fd1f40dc>] ssb_pcihost_probe+0x9c/0x110 [ssb]
> [<c1276c8f>] pci_device_probe+0x6f/0xc0
> [<c11bdb55>] ? sysfs_create_link+0x25/0x40
> [<c131d8b9>] driver_probe_device+0x79/0x360
> [<c1276512>] ? pci_match_device+0xb2/0xc0
> [<c131dc51>] __driver_attach+0x71/0x80
> [<c131dbe0>] ? __device_attach+0x40/0x40
> [<c131bd87>] bus_for_each_dev+0x47/0x80
> [<c131d3ae>] driver_attach+0x1e/0x20
> [<c131dbe0>] ? __device_attach+0x40/0x40
> [<c131d007>] bus_add_driver+0x157/0x230
> [<c131e219>] driver_register+0x59/0xe0
> ...
>
> Fix this by checking chip->dev pointer against NULL first. Also we can now
> remove redundant check in acpi_gpiochip_request/free_interrupts().
Acked-by: Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio / ACPI: Don't crash on NULL chip->dev
2014-03-31 12:16 [PATCH] gpio / ACPI: Don't crash on NULL chip->dev Mika Westerberg
2014-03-31 16:25 ` Sabrina Dubroca
2014-03-31 19:33 ` Alexandre Courbot
@ 2014-04-10 16:06 ` Linus Walleij
2 siblings, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2014-04-10 16:06 UTC (permalink / raw)
To: Mika Westerberg
Cc: Alexandre Courbot, Sabrina Dubroca, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org
On Mon, Mar 31, 2014 at 2:16 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> Commit aa92b6f689ac (gpio / ACPI: Allocate ACPI specific data directly in
> acpi_gpiochip_add()) moved ACPI handle checking to acpi_gpiochip_add() but
> forgot to check whether chip->dev is NULL before dereferencing it.
>
> Since chip->dev pointer is optional we can end up with crash like following:
Sorry for taking so long before getting to it, patch applied for fixes.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-04-10 16:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-31 12:16 [PATCH] gpio / ACPI: Don't crash on NULL chip->dev Mika Westerberg
2014-03-31 16:25 ` Sabrina Dubroca
2014-03-31 19:33 ` Alexandre Courbot
2014-04-10 16:06 ` Linus Walleij
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).