Possible race in pc87413_wdt.ko

Possible race in pc87413_wdt.ko

[Anton Volkov]

Hello.

While searching for races in the Linux kernel I've come across 
"drivers/watchdog/pc87413_wdt.ko" module. Here is a question that I came 
up with while analyzing results. Lines are given using the info from 
Linux v4.12.

Consider the following case:

Thread 1:                          Thread 2:
pc87413_init
    misc_register(&pc87413_miscdev)
-> pc87413_get_swc_base_addr       pc87413_open
                                    -> pc87413_refresh
                                       -> pc87413_swc_bank3
      swc_base_addr = ...                  <read access to swc_base_addr>
      (pc87413_wdt.c: line 133)            (pc87413_wdt.c: line 146)

So in this case preemptive registration of the device leads to a 
possibility of race between the initialization process and a callback to 
the registered device.

Is this race feasible from your point of view? And if it is, is it 
possible to move the device registration a bit further down in the 
pc87413_init function?

Thank you for your time.

-- Anton Volkov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
e-mail: avolkov@ispras.ru

Re: Possible race in pc87413_wdt.ko

[Guenter Roeck]

On 08/07/2017 06:22 AM, Anton Volkov wrote:
> Hello.
> 
> While searching for races in the Linux kernel I've come across "drivers/watchdog/pc87413_wdt.ko" module. Here is a question that I came up with while analyzing results. Lines are given using the info from Linux v4.12.
> 
> Consider the following case:
> 
> Thread 1:                          Thread 2:
> pc87413_init
>     misc_register(&pc87413_miscdev)
> -> pc87413_get_swc_base_addr       pc87413_open
>                                     -> pc87413_refresh
>                                        -> pc87413_swc_bank3
>       swc_base_addr = ...                  <read access to swc_base_addr>
>       (pc87413_wdt.c: line 133)            (pc87413_wdt.c: line 146)
> 
> So in this case preemptive registration of the device leads to a possibility of race between the initialization process and a callback to the registered device.
> 
> Is this race feasible from your point of view? And if it is, is it possible to move the device registration a bit further down in the pc87413_init function?
> 

Yes, the race is feasible, and it is possible to move the device registration function
(though the preferred solution would be to convert the driver to use the watchdog
subsystem). The code looks pretty bad as written.

Just not sure if it is worth bothering about it. I suspect no on is using that driver
anymore (the datasheet is from 2001). Might as well just declare it obsolete and
wait for someone to scream.

Guenter