From e9595f84a6273dffc5b75564d9b12a77630c529e Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Mon, 31 Mar 2014 15:16:49 +0300 Subject: gpio / ACPI: Don't crash on NULL chip->dev 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: [] 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:[] 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: [] ? ssb_pcie_mdio_write+0xa0/0xd0 [ssb] [] gpiochip_add+0xee/0x300 [] ? ssb_pcicore_serdes_workaround+0xfd/0x140 [ssb] [] ssb_gpio_init+0x89/0xa0 [ssb] [] ssb_attach_queued_buses+0xc8/0x2d0 [ssb] [] ssb_bus_register+0x185/0x1f0 [ssb] [] ? ssb_pci_xtal+0x220/0x220 [ssb] [] ssb_bus_pcibus_register+0x2c/0x80 [ssb] [] ssb_pcihost_probe+0x9c/0x110 [ssb] [] pci_device_probe+0x6f/0xc0 [] ? sysfs_create_link+0x25/0x40 [] driver_probe_device+0x79/0x360 [] ? pci_match_device+0xb2/0xc0 [] __driver_attach+0x71/0x80 [] ? __device_attach+0x40/0x40 [] bus_for_each_dev+0x47/0x80 [] driver_attach+0x1e/0x20 [] ? __device_attach+0x40/0x40 [] bus_add_driver+0x157/0x230 [] 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 Signed-off-by: Mika Westerberg Tested-by: Sabrina Dubroca Acked-by: Alexandre Courbot Tested-by: Josh Boyer Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib-acpi.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'drivers/gpio/gpiolib-acpi.c') diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index bf0f8b47669..d5be56fe689 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; -- cgit v1.2.3-70-g09d2 From b5539fa2d59d697b7b8e28b4d08da844ff60f7cf Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Tue, 1 Apr 2014 13:03:00 +0300 Subject: gpio / ACPI: Prevent potential wrap of GPIO value on OpRegion read Dan Carpenter's static code checker reports: The patch 473ed7be0da0: "gpio / ACPI: Add support for ACPI GPIO operation regions" from Mar 14, 2014, leads to the following static checker warning: drivers/gpio/gpiolib-acpi.c:454 acpi_gpio_adr_space_handler() warn: should 'gpiod_get_raw_value(desc) << i' be a 64 bit type? This is due the fact that *value is of type u64 and gpiod_get_raw_value() returns int. Since i can be larger than 31, it is possible that the value returned gets wrapped. Fix this by casting the return of gpiod_get_raw_value() to u64 first before shift. Reported-by: Dan Carpenter Signed-off-by: Mika Westerberg Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib-acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpio/gpiolib-acpi.c') diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index d5be56fe689..401add28933 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -451,7 +451,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address, if (function == ACPI_WRITE) gpiod_set_raw_value(desc, !!((1 << i) & *value)); else - *value |= gpiod_get_raw_value(desc) << i; + *value |= (u64)gpiod_get_raw_value(desc) << i; } out: -- cgit v1.2.3-70-g09d2