Re: [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU

[Bartosz Golaszewski]

On Fri, Feb 16, 2024 at 11:31 AM Marek Szyprowski
<m.szyprowski@xxxxxxxxxxx> wrote:
>

[snip]

>
> Here 'gc' is left uninitialized and nukes if GPIO DEBUGs are enabled.
> Here is an example (captured on today's linux-next):
>
> 8<--- cut here ---
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000000 when read
> [00000000] *pgd=00000000
> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc4-next-20240216 #8096
> Hardware name: Samsung Exynos (Flattened Device Tree)
> PC is at gpiolib_cdev_register+0xd4/0x170
> LR is at chainhash_table+0x784/0x20000
> pc : [<c05dbe54>]    lr : [<c18bb74c>]    psr: 20000013
> ...
> Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> Control: 10c5387d  Table: 4000404a  DAC: 00000051
> Register r0 information: non-slab/vmalloc memory
> Register r1 information: NULL pointer
> Register r2 information: non-paged memory
> Register r3 information: non-paged memory
> Register r4 information: slab kmalloc-1k start c1e5f800 pointer offset 0
> size 1024
> Register r5 information: NULL pointer
> Register r6 information: non-paged memory
> Register r7 information: slab kmalloc-1k start c1e5f800 pointer offset
> 952 size 1024
> Register r8 information: NULL pointer
> Register r9 information: slab kmalloc-1k start c1e5f800 pointer offset
> 960 size 1024
> Register r10 information: non-paged memory
> Register r11 information: non-slab/vmalloc memory
> Register r12 information: NULL pointer
> Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
> Stack: (0xf082db90 to 0xf082e000)
> ...
>   gpiolib_cdev_register from gpiochip_setup_dev+0x44/0xb0
>   gpiochip_setup_dev from gpiochip_add_data_with_key+0x9ac/0xaac
>   gpiochip_add_data_with_key from devm_gpiochip_add_data_with_key+0x20/0x5c
>   devm_gpiochip_add_data_with_key from samsung_pinctrl_probe+0x938/0xb18
>   samsung_pinctrl_probe from platform_probe+0x5c/0xb8
>   platform_probe from really_probe+0xe0/0x400
>   really_probe from __driver_probe_device+0x9c/0x1f4
>   __driver_probe_device from driver_probe_device+0x30/0xc0
>   driver_probe_device from __device_attach_driver+0xa8/0x120
>   __device_attach_driver from bus_for_each_drv+0x80/0xcc
>   bus_for_each_drv from __device_attach+0xac/0x1fc
>   __device_attach from bus_probe_device+0x8c/0x90
>   bus_probe_device from device_add+0x5d4/0x7fc
>   device_add from of_platform_device_create_pdata+0x94/0xc4
>   of_platform_device_create_pdata from of_platform_bus_create+0x1f8/0x4c0
>   of_platform_bus_create from of_platform_bus_create+0x268/0x4c0
>   of_platform_bus_create from of_platform_populate+0x80/0x110
>   of_platform_populate from of_platform_default_populate_init+0xd4/0xec
>   of_platform_default_populate_init from do_one_initcall+0x64/0x2fc
>   do_one_initcall from kernel_init_freeable+0x1c4/0x228
>   kernel_init_freeable from kernel_init+0x1c/0x12c
>   kernel_init from ret_from_fork+0x14/0x28
> Exception stack(0xf082dfb0 to 0xf082dff8)
> ...
> ---[ end trace 0000000000000000 ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> ---[ end Kernel panic - not syncing: Attempted to kill init!
> exitcode=0x0000000b ]---
>
>
> Probably the easiest way to fix this issue is to replace chip_dbg with
> the following dev_dbg() call:
>
> dev_dbg(&gdev->dev, "(%s): added GPIO chardev (%d:%d)\n", gdev->label,
> MAJOR(devt), gdev->id);
>

Thanks for the report. Surprisingly there are no warnings about that
with GCC. The best way is to use srcu_dereference() here and keep the
same log message. Patch is on the way.

Bart