Re: [PATCH] gpio: Fix crash due to registration race

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Fri, Aug 31, 2018 at 9:04 AM Vincent Whitchurch
<vincent.whitchurch@xxxxxxxx> wrote:

> gpiochip_add_data_with_key() adds the gpiochip to the gpio_devices list
> before of_gpiochip_add() is called, but it's only the latter which sets
> the ->of_xlate function pointer.  gpiochip_find() can be called by
> someone else between these two actions, and it can find the chip and
> call of_gpiochip_match_node_and_xlate() which leads to the following
> crash due to a NULL ->of_xlate().
>
>  Unhandled prefetch abort: page domain fault (0x01b) at 0x00000000
>  Modules linked in: leds_gpio(+) gpio_generic(+)
>  CPU: 0 PID: 830 Comm: insmod Not tainted 4.18.0+ #43
>  Hardware name: ARM-Versatile Express
>  PC is at   (null)
>  LR is at of_gpiochip_match_node_and_xlate+0x2c/0x38
>  Process insmod (pid: 830, stack limit = 0x(ptrval))
>   (of_gpiochip_match_node_and_xlate) from  (gpiochip_find+0x48/0x84)
>   (gpiochip_find) from  (of_get_named_gpiod_flags+0xa8/0x238)
>   (of_get_named_gpiod_flags) from  (gpiod_get_from_of_node+0x2c/0xc8)
>   (gpiod_get_from_of_node) from  (devm_fwnode_get_index_gpiod_from_child+0xb8/0x144)
>   (devm_fwnode_get_index_gpiod_from_child) from  (gpio_led_probe+0x208/0x3c4 [leds_gpio])
>   (gpio_led_probe [leds_gpio]) from  (platform_drv_probe+0x48/0x9c)
>   (platform_drv_probe) from  (really_probe+0x1d0/0x3d4)
>   (really_probe) from  (driver_probe_device+0x78/0x1c0)
>   (driver_probe_device) from  (__driver_attach+0x120/0x13c)
>   (__driver_attach) from  (bus_for_each_dev+0x68/0xb4)
>   (bus_for_each_dev) from  (bus_add_driver+0x1a8/0x268)
>   (bus_add_driver) from  (driver_register+0x78/0x10c)
>   (driver_register) from  (do_one_initcall+0x54/0x1fc)
>   (do_one_initcall) from  (do_init_module+0x64/0x1f4)
>   (do_init_module) from  (load_module+0x2198/0x26ac)
>   (load_module) from  (sys_finit_module+0xe0/0x110)
>   (sys_finit_module) from  (ret_fast_syscall+0x0/0x54)
>
> One way to fix this would be to rework the hairy registration sequence
> in gpiochip_add_data_with_key(), but since I'd probably introduce a
> couple of new bugs if I attempted that, simply add a check for a
> non-NULL of_xlate function pointer in
> of_gpiochip_match_node_and_xlate().  This works since the driver looking
> for the gpio will simply fail to find the gpio and defer its probe and
> be reprobed when the driver which is registering the gpiochip has fully
> completed its probe.
>
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@xxxxxxxx>

Patch applied for fixes!

Weird that we haven't run into this before.

Yours,
Linus Walleij