Hello.
While searching for races in the Linux kernel I've come across
"drivers/input/touchscreen/ucb1400_ts.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:
ucb1400_suspend
->ucb1400_ts_start
ucb->stopped = false
enable_irq()
ucb1400_resume
->ucb1400_ts_stop ucb1400_irq
ucb->stopped = true while(!ucb->stopped && ...)
(ucb1400_ts.c: line 230) (ucb1400_ts.c: line 202)
disable_irq()
The value of ucb->stopped may be changed in the midst of 'while' loop
iterations or prevent all of them from happening. Is this feasible from
your point of view? If so, is it a benign race or is it serious?
Thank you for your time.
-- Anton Volkov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
e-mail: [email protected]
Hi Anton,
On Tue, Aug 15, 2017 at 04:46:25PM +0300, Anton Volkov wrote:
> Hello.
>
> While searching for races in the Linux kernel I've come across
> "drivers/input/touchscreen/ucb1400_ts.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:
> ucb1400_suspend
> ->ucb1400_ts_start
> ucb->stopped = false
> enable_irq()
>
> ucb1400_resume
> ->ucb1400_ts_stop ucb1400_irq
> ucb->stopped = true while(!ucb->stopped && ...)
> (ucb1400_ts.c: line 230) (ucb1400_ts.c: line 202)
> disable_irq()
>
> The value of ucb->stopped may be changed in the midst of 'while'
> loop iterations or prevent all of them from happening. Is this
> feasible from your point of view? If so, is it a benign race or is
> it serious?
Well, I guess nobody is using that driver in mainline, or at least not
with platforms that do system suspend. The suspend is supposed to call
ucb1400_ts_stop(), not ucb1400_ts_start(), and resume is messed up as
well.
We need to fix it. Once it is done, then it should look better. The
ucb->stopped can change in the middle of while loop, and that should
cause the interrupt handler to stop running. The "stop" uses
disable_irq() and thus will wait for the interrupt handler to finish
before continuing.
Thanks.
--
Dmitry