NULL pointer dereference when 'rmmod gpio_lynxpoint'

NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Anatol Pomozov]

Hi

I am trying to 'rmmod gpio_lynxpoint' module at my Chromebox with Arch
4.5.1 kernel and get this oops:

[  118.801173] BUG: unable to handle kernel NULL pointer dereference
at 0000000000000078
[  118.801228] IP: [<ffffffff8105401f>] apic_ack_edge+0x2f/0x60
[  118.801267] PGD 74193067 PUD 74192067 PMD 0
[  118.801297] Oops: 0000 [#1] PREEMPT SMP
[  118.801326] Modules linked in: rfcomm snd_hda_codec_hdmi intel_rapl
arc4 x86_pkg_temp_thermal intel_powerclamp coretemp ath9k kvm_intel
ath9k_common kvm ath9k_hw irqbypass iTCO_wdt iTCO_vendor_support evdev
mac_hid ath crct10dif_pclmul crc32_pclmul mac80211 crc32c_intel
ghash_clmulni_intel cryptd i915 snd_hda_codec_realtek ath3k btusb
snd_hda_codec_generic btrtl drm_kms_helper btbcm snd_hda_intel btintel
pcspkr bluetooth cfg80211 snd_hda_codec i2c_i801 drm snd_hda_core
intel_gtt syscopyarea sysfillrect sysimgblt lpc_ich fb_sys_fops
dw_dmac snd_hwdep i2c_algo_bit rfkill r8169 snd_pcm mei_me mii
dw_dmac_core snd_timer mei snd soundcore shpchp tpm_tis fan tpm fjes
8250_dw i2c_designware_platform gpio_lynxpoint(-) spi_pxa2xx_platform
i2c_designware_core video thermal button processor sch_fq_codel
ip_tables
[  118.801846]  x_tables ext4 crc16 mbcache jbd2 sd_mod ahci libahci
sdhci_acpi libata sdhci led_class mmc_core scsi_mod xhci_pci xhci_hcd
usbcore usb_common
[  118.801952] CPU: 1 PID: 368 Comm: rmmod Not tainted 4.5.1-1-ARCH #1
[  118.801988] Hardware name: Google Panther/Panther, BIOS MattDevo 04/03/2016
[  118.802027] task: ffff88006acfa7c0 ti: ffff8800629c4000 task.ti:
ffff8800629c4000
[  118.802068] RIP: 0010:[<ffffffff8105401f>]  [<ffffffff8105401f>]
apic_ack_edge+0x2f/0x60
[  118.802119] RSP: 0018:ffff8800629c7d60  EFLAGS: 00010046
[  118.802150] RAX: 0000000000000000 RBX: ffff880100500fc0 RCX: 0000000000000000
[  118.802189] RDX: ffff880100500fc0 RSI: 0000000000000046 RDI: ffff880100500280
[  118.802228] RBP: ffff8800629c7d68 R08: 0000000000000000 R09: ffff880100000000
[  118.802267] R10: 0000000000000000 R11: ffff880073ecac20 R12: ffff88010043e428
[  118.802308] R13: 0000000000000001 R14: 0000000000000000 R15: 0000000001ccf010
[  118.802349] FS:  00007f7a78de7700(0000) GS:ffff880100300000(0000)
knlGS:0000000000000000
[  118.802394] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  118.802425] CR2: 0000000000000078 CR3: 0000000074190000 CR4: 00000000000406e0
[  118.802464] Stack:
[  118.802477]  ffff88010043e400 ffff8800629c7d78 ffffffff810d4c24
ffff8800629c7da8
[  118.802524]  ffffffff810d5af9 0000000000000000 0000000100000000
ffff88010043e400
[  118.802571]  0000000000000000 ffff8800629c7de8 ffffffff810d5b65
0000000000000246
[  118.802618] Call Trace:
[  118.802639]  [<ffffffff810d4c24>] irq_chip_ack_parent+0x14/0x20
[  118.802674]  [<ffffffff810d5af9>] __irq_do_set_handler+0x159/0x170
[  118.802710]  [<ffffffff810d5b65>] __irq_set_handler+0x55/0x90
[  118.802747]  [<ffffffff81311d7f>] gpiochip_remove+0x18f/0x1e0
[  118.802782]  [<ffffffffa0101397>] lp_gpio_remove+0x27/0x30 [gpio_lynxpoint]
[  118.802822]  [<ffffffff81402b78>] platform_drv_remove+0x28/0x40
[  118.802860]  [<ffffffff81400281>] __device_release_driver+0xa1/0x160
[  118.802898]  [<ffffffff81400e14>] driver_detach+0xb4/0xc0
[  118.802930]  [<ffffffff813ffc25>] bus_remove_driver+0x55/0xd0
[  118.802964]  [<ffffffff8140167c>] driver_unregister+0x2c/0x50
[  118.802997]  [<ffffffff81402c42>] platform_driver_unregister+0x12/0x20
[  118.803035]  [<ffffffffa010182b>] lp_gpio_exit+0x10/0x7e5 [gpio_lynxpoint]
[  118.803074]  [<ffffffff810fdbb1>] SyS_delete_module+0x1b1/0x260
[  118.803109]  [<ffffffff8100364e>] ? exit_to_usermode_loop+0x5e/0xc0
[  118.803146]  [<ffffffff815ad6ae>] entry_SYSCALL_64_fastpath+0x12/0x6d
[  118.803182] Code: 00 55 48 85 ff 48 89 e5 53 48 89 fb 74 47 48 89
fa eb 03 48 89 c2 48 8b 42 28 48 85 c0 75 f4 48 8b 7a 30 65 48 8b 05
a9 70 fb 7e <48> 8b 70 78 f7 d6 e8 86 f9 ff ff 48 89 df e8 1e 4d 08 00
48 8b
[  118.803396] RIP  [<ffffffff8105401f>] apic_ack_edge+0x2f/0x60
[  118.805687]  RSP <ffff8800629c7d60>
[  118.807981] CR2: 0000000000000078
[  118.823530] ---[ end trace 540a61ea62273024 ]---
[  118.823534] note: rmmod[368] exited with preempt_count 1

Re: NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Linus Walleij]

On Sun, May 1, 2016 at 2:33 AM, Anatol Pomozov <anatol.pomozov@gmail.com> wrote:

> I am trying to 'rmmod gpio_lynxpoint' module at my Chromebox with Arch
> 4.5.1 kernel and get this oops:
>
> [  118.801173] BUG: unable to handle kernel NULL pointer dereference
> at 0000000000000078
> [  118.801228] IP: [<ffffffff8105401f>] apic_ack_edge+0x2f/0x60

Mathias Nyman is the maintainer of this driver. Interesting that
a recent Chromebox is using Lynxpoint!

Are you Cc:ing Laxman because you think the addition of
devm_gpiochip_add_data() handling is causing it?

I think this looks much more related to the fact that we're removing
an irqchip and it's not really working.

GPIO drivers with backing irqchips are providing resources to other
drivers, they are seldom really removeable.

Maybe this driver should be made a bool?

Yours,
Linus Walleij

Re: NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Anatol Pomozov]

Hi

+Matt, the maintainer of Coreboot firmware that I use at this device.

On Sun, May 1, 2016 at 1:38 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sun, May 1, 2016 at 2:33 AM, Anatol Pomozov <anatol.pomozov@gmail.com> wrote:
>
>> I am trying to 'rmmod gpio_lynxpoint' module at my Chromebox with Arch
>> 4.5.1 kernel and get this oops:
>>
>> [  118.801173] BUG: unable to handle kernel NULL pointer dereference
>> at 0000000000000078
>> [  118.801228] IP: [<ffffffff8105401f>] apic_ack_edge+0x2f/0x60
>
> Mathias Nyman is the maintainer of this driver. Interesting that
> a recent Chromebox is using Lynxpoint!
>
> Are you Cc:ing Laxman because you think the addition of
> devm_gpiochip_add_data() handling is causing it?

Yeah. I was looking at the history of recent changes in that file and
just CC a few people.

Starting from 4.2/4.3 kernel release I see numerous problems with this
driver. For example there are regular crashes at the boot time when
tries to load this driver https://goo.gl/photos/fGk2U6PgUFtxUk9w5    I
have to stick with Arch 4.1 kernel that does not have this problem.

> I think this looks much more related to the fact that we're removing
> an irqchip and it's not really working.
>
> GPIO drivers with backing irqchips are providing resources to other
> drivers, they are seldom really removeable.
>
> Maybe this driver should be made a bool?

Re: NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Linus Walleij]

On Sun, May 1, 2016 at 4:36 PM, Anatol Pomozov <anatol.pomozov@gmail.com> wrote:
> On Sun, May 1, 2016 at 1:38 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Sun, May 1, 2016 at 2:33 AM, Anatol Pomozov <anatol.pomozov@gmail.com> wrote:

> Starting from 4.2/4.3 kernel release I see numerous problems with this
> driver. For example there are regular crashes at the boot time when
> tries to load this driver https://goo.gl/photos/fGk2U6PgUFtxUk9w5    I
> have to stick with Arch 4.1 kernel that does not have this problem.

Hm atleast it is not due to my recent refactorings of GPIO then.

Any chance you can run a bisect and nail the commit?

Yours,
Linus Walleij

Re: NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Anatol Pomozov]

Hi

On Sun, May 1, 2016 at 11:30 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sun, May 1, 2016 at 4:36 PM, Anatol Pomozov <anatol.pomozov@gmail.com> wrote:
>> On Sun, May 1, 2016 at 1:38 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>>> On Sun, May 1, 2016 at 2:33 AM, Anatol Pomozov <anatol.pomozov@gmail.com> wrote:
>
>> Starting from 4.2/4.3 kernel release I see numerous problems with this
>> driver. For example there are regular crashes at the boot time when
>> tries to load this driver https://goo.gl/photos/fGk2U6PgUFtxUk9w5    I
>> have to stick with Arch 4.1 kernel that does not have this problem.
>
> Hm atleast it is not due to my recent refactorings of GPIO then.
>
> Any chance you can run a bisect and nail the commit?

I compiled lynx gpio into kernel instead of compiling it as a module.
And the system got more stable - I rebooted my devices ~50 times
already and I do not see the boot problem I had before. 'CONFIG_..=y'
definitely makes the situation better.

As of bisecting - it will require time do find the commit. I might do
it if I find some spare time.

Re: NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Linus Walleij]

On Tue, May 3, 2016 at 5:23 AM, Anatol Pomozov <anatol.pomozov@gmail.com> wrote:
> On Sun, May 1, 2016 at 11:30 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Sun, May 1, 2016 at 4:36 PM, Anatol Pomozov <anatol.pomozov@gmail.com> wrote:
>>> On Sun, May 1, 2016 at 1:38 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>>>> On Sun, May 1, 2016 at 2:33 AM, Anatol Pomozov <anatol.pomozov@gmail.com> wrote:
>>
>>> Starting from 4.2/4.3 kernel release I see numerous problems with this
>>> driver. For example there are regular crashes at the boot time when
>>> tries to load this driver https://goo.gl/photos/fGk2U6PgUFtxUk9w5    I
>>> have to stick with Arch 4.1 kernel that does not have this problem.
>>
>> Hm atleast it is not due to my recent refactorings of GPIO then.
>>
>> Any chance you can run a bisect and nail the commit?
>
> I compiled lynx gpio into kernel instead of compiling it as a module.
> And the system got more stable - I rebooted my devices ~50 times
> already and I do not see the boot problem I had before. 'CONFIG_..=y'
> definitely makes the situation better.
>
> As of bisecting - it will require time do find the commit. I might do
> it if I find some spare time.

If nothing else works I'm happy to accept a patch turning the
driver into a bool and removing all module handling code.

Yours,
Linus Walleij

Re: NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Mika Westerberg]

On Wed, May 11, 2016 at 07:00:18AM -0700, Anatol Pomozov wrote:
>    Hi
>    I tried to read the gpio handling code, but I do not see any obvious
>    reason for this "insert module" crash. And I had no time to do history
>    bisect.
>    On Wed, May 11, 2016 at 2:22 AM, Linus Walleij
>    <[1]linus.walleij@linaro.org> wrote:
> 
>      > I compiled lynx gpio into kernel instead of compiling it as a
>      module.
>      > And the system got more stable - I rebooted my devices ~50 times
>      > already and I do not see the boot problem I had before.
>      'CONFIG_..=y'
>      > definitely makes the situation better.
>      >
>      > As of bisecting - it will require time do find the commit. I might
>      do
>      > it if I find some spare time.
>      If nothing else works I'm happy to accept a patch turning the
>      driver into a bool and removing all module handling code.
> 
>    Before doing this I would love to get some feedback from the driver
>    maintainer. Mathias, do you see this issue with the module?
>    Also I am going to change Arch Linux kernel config to compile this
>    driver as 'y'.

Let's first analyze why it crashes in the first place. Can you give me
detailed instructions how can I reproduce this?

Re: NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Anatol Pomozov]

Hi

On Wed, May 11, 2016 at 7:02 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Wed, May 11, 2016 at 07:00:18AM -0700, Anatol Pomozov wrote:
>>    Hi
>>    I tried to read the gpio handling code, but I do not see any obvious
>>    reason for this "insert module" crash. And I had no time to do history
>>    bisect.
>>    On Wed, May 11, 2016 at 2:22 AM, Linus Walleij
>>    <[1]linus.walleij@linaro.org> wrote:
>>
>>      > I compiled lynx gpio into kernel instead of compiling it as a
>>      module.
>>      > And the system got more stable - I rebooted my devices ~50 times
>>      > already and I do not see the boot problem I had before.
>>      'CONFIG_..=y'
>>      > definitely makes the situation better.
>>      >
>>      > As of bisecting - it will require time do find the commit. I might
>>      do
>>      > it if I find some spare time.
>>      If nothing else works I'm happy to accept a patch turning the
>>      driver into a bool and removing all module handling code.
>>
>>    Before doing this I would love to get some feedback from the driver
>>    maintainer. Mathias, do you see this issue with the module?
>>    Also I am going to change Arch Linux kernel config to compile this
>>    driver as 'y'.
>
> Let's first analyze why it crashes in the first place. Can you give me
> detailed instructions how can I reproduce this?

I use Chromebox with Matt's coreboot firmware and Arch Linux that
currently compiles LYNX gpio as a module.

There are 2 crashes:
1) ~25% chance of crash at boot time. Just boot the device and the
system crashes when tries to probe the driver
2) system crashes when try to 'rmmod'. See stack trace in the first
message of this thread.

Re: NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Mika Westerberg]

On Thu, May 12, 2016 at 06:35:20AM -0700, Anatol Pomozov wrote:
> I use Chromebox with Matt's coreboot firmware and Arch Linux that
> currently compiles LYNX gpio as a module.
>
> There are 2 crashes:
> 1) ~25% chance of crash at boot time. Just boot the device and the
> system crashes when tries to probe the driver
> 2) system crashes when try to 'rmmod'. See stack trace in the first
> message of this thread.

I tried pretty much the same on a Broadwell based machine (uses the same
GPIO driver) and I did not see any crashes. I don't have Haswell based
machines around anymore.

Since you seem to be able get it crashing pretty easily, I wonder if you
can try following to debug the first crash:

 1. Blacklist gpio-lynxpoint
 2. Boot the system
 3. Send me contents of /proc/interrupts and /sys/kernel/debug/gpio
 4. Run 'insmod gpio-lynxpoint.ko'
 5. Send me full dmesg of the crash (if you can get it).

And the second crash:

 1. Remoge gpio-lynxpoint from blacklist
 2. Boot the system
 3. Run 'rmmod gpio-lynxpoint'
 4. Send me full dmesg of the crash (if you can get it).

Hopefully with that information we can find root cause of both crashes
(which may be the same).

Re: NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Mathias Nyman]

Hi

On 11.05.2016 17:00, Anatol Pomozov wrote:
> Hi
>
> I tried to read the gpio handling code, but I do not see any obvious reason for this "insert module" crash. And I had no time to do history bisect.
>
> On Wed, May 11, 2016 at 2:22 AM, Linus Walleij <linus.walleij@linaro.org <mailto:linus.walleij@linaro.org>> wrote:
>
>     > I compiled lynx gpio into kernel instead of compiling it as a module.
>     > And the system got more stable - I rebooted my devices ~50 times
>     > already and I do not see the boot problem I had before. 'CONFIG_..=y'
>     > definitely makes the situation better.
>     >
>     > As of bisecting - it will require time do find the commit. I might do
>     > it if I find some spare time.
>
>     If nothing else works I'm happy to accept a patch turning the
>     driver into a bool and removing all module handling code.
>
>
> Before doing this I would love to get some feedback from the driver maintainer. Mathias, do you see this issue with the module?
>
> Also I am going to change Arch Linux kernel config to compile this driver as 'y'.

I haven't been looking at this code for some time, Mika has mostly been taking care of it lately.

Driver was originally written as bool (not module) partly because we wanted it to load earlier as
we suspected some other drivers depend on gpio interrupts, and partly because the IO port resources
overlapped with motherboard resources that pnpacpi would request before gpio-lynxpoint.

As driver was written with the original assumption of not being a module, there
might be that the remove codepaths in gpio-lynxopoint are not properly calling
gpiochip_remove (that should call gpiochip_irqchip_remove etc) on driver unload,
or not checking if those return errors, or ourself not returning proper errors on removes that should fail.

An other possible issue could be that if hw reduced acpi stuff that expects os to have a interrupt handler for
gpio interrupts triggered in firmware. Not sure if that eneded up being used anywhere, but I can imagine there being issues
if we remove the interrupt handling for those.

Anyways, I don't mind turning it back to a bool, but I agree with Mika that we really should investigate the cause
behind this and not just do it because it removes the symptoms.

-Mathias
    

Re: NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Linus Walleij]

On Thu, May 12, 2016 at 12:34 PM, Mathias Nyman
<mathias.nyman@linux.intel.com> wrote:

> As driver was written with the original assumption of not being a module,
> there
> might be that the remove codepaths in gpio-lynxopoint are not properly
> calling
> gpiochip_remove (that should call gpiochip_irqchip_remove etc) on driver
> unload,
> or not checking if those return errors, or ourself not returning proper
> errors on removes that should fail.

We have devm_gpiochip_add_data() these days, so you don't need
to call gpiochip_remove() explicitly if you use that.

Yours,
Linus Walleij

Re: NULL pointer dereference when 'rmmod gpio_lynxpoint'

[Anatol Pomozov]

...resending it as a plain text


I tried to read the gpio handling code, but I do not see any obvious
reason for this "insert module" crash. And I had no time to do the
history bisect.

On Wed, May 11, 2016 at 2:22 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> I compiled lynx gpio into kernel instead of compiling it as a module.
>> And the system got more stable - I rebooted my devices ~50 times
>> already and I do not see the boot problem I had before. 'CONFIG_..=y'
>> definitely makes the situation better.
>>
>> As of bisecting - it will require time do find the commit. I might do
>> it if I find some spare time.
>
> If nothing else works I'm happy to accept a patch turning the
> driver into a bool and removing all module handling code.

Before doing this I would love to get some feedback from the driver
maintainer. Mathias, do you see this issue with the module?

Also I am going to change Arch Linux kernel config to compile this
driver as 'y'.