nwbutton: lost wakeup in button_read() -- worth fixing, or is this driver done?

nwbutton: lost wakeup in button_read() -- worth fixing, or is this driver done?

[old king]

Hi,

button_read() in drivers/char/nwbutton.c sleeps unconditionally: nothing is
tested before schedule(), not even whether the buffer is empty. If
button_sequence_finished() wakes button_wait_queue in the window between
read() being entered and prepare_to_wait() queuing the reader, that wakeup
is lost and the reader sleeps until some later, unrelated button sequence
completes. The race predates commit eb831743f2b4, whose changelog already called
the code "inherently racy". I have reproduced it under QEMU and prepared a fix:
a sequence counter captured before queuing and re-checked after.

Before I send that: is this driver still worth fixing? footbridge is
deprecated and its Kconfig says support goes away in 2027 unless remaining
users speak up. Every change to nwbutton.c in git history has been a
treewide cleanup rather than anything a user asked for, and
button_add_callback()/button_del_callback() have no callers in the tree. I
have no NetWinder hardware and no way to tell whether anyone else does.

The lost wakeup is also not the only problem. The same seven-line read()
ignores the caller's count -- read(fd, buf, 0) returns 2 and writes two
bytes -- and never checks signal_pending() after schedule(), so a signal
returns the previous sequence's data instead of -EINTR. I verified both
under QEMU as well, but in a single run only -- unlike the wakeup, I have
not repeated them independently.

Should I send fixes, or a patch removing the driver? Happy to write either.

Best regards,
Fan Ye

Re: nwbutton: lost wakeup in button_read() -- worth fixing, or is this driver done?

[Greg KH]

On Sat, Aug 22, 2026 at 11:01:38AM -0500, old king wrote:
> Hi,
> 
> button_read() in drivers/char/nwbutton.c sleeps unconditionally: nothing is
> tested before schedule(), not even whether the buffer is empty. If
> button_sequence_finished() wakes button_wait_queue in the window between
> read() being entered and prepare_to_wait() queuing the reader, that wakeup
> is lost and the reader sleeps until some later, unrelated button sequence
> completes. The race predates commit eb831743f2b4, whose changelog already called
> the code "inherently racy". I have reproduced it under QEMU and prepared a fix:
> a sequence counter captured before queuing and re-checked after.
> 
> Before I send that: is this driver still worth fixing? footbridge is
> deprecated and its Kconfig says support goes away in 2027 unless remaining
> users speak up. Every change to nwbutton.c in git history has been a
> treewide cleanup rather than anything a user asked for, and
> button_add_callback()/button_del_callback() have no callers in the tree. I
> have no NetWinder hardware and no way to tell whether anyone else does.

If you don't have the hardware to test it, don't worry about the driver
at all.

thanks,

greg k-h

Re: nwbutton: lost wakeup in button_read() -- worth fixing, or is this driver done?

[Greg KH]

On Sat, Aug 22, 2026 at 06:06:13PM +0200, Greg KH wrote:
> On Sat, Aug 22, 2026 at 11:01:38AM -0500, old king wrote:
> > Hi,
> > 
> > button_read() in drivers/char/nwbutton.c sleeps unconditionally: nothing is
> > tested before schedule(), not even whether the buffer is empty. If
> > button_sequence_finished() wakes button_wait_queue in the window between
> > read() being entered and prepare_to_wait() queuing the reader, that wakeup
> > is lost and the reader sleeps until some later, unrelated button sequence
> > completes. The race predates commit eb831743f2b4, whose changelog already called
> > the code "inherently racy". I have reproduced it under QEMU and prepared a fix:
> > a sequence counter captured before queuing and re-checked after.
> > 
> > Before I send that: is this driver still worth fixing? footbridge is
> > deprecated and its Kconfig says support goes away in 2027 unless remaining
> > users speak up. Every change to nwbutton.c in git history has been a
> > treewide cleanup rather than anything a user asked for, and
> > button_add_callback()/button_del_callback() have no callers in the tree. I
> > have no NetWinder hardware and no way to tell whether anyone else does.
> 
> If you don't have the hardware to test it, don't worry about the driver
> at all.

I don't think it can even build, given that there is no definition for
machine_is_netwinder() anywhere in the tree that I can see.  So maybe
many of these drivers can just be removed now.  Arnd, any ideas?

thanks,

greg k-h

Re: nwbutton: lost wakeup in button_read() -- worth fixing, or is this driver done?

[Arnd Bergmann]

On Sat, Aug 22, 2026, at 18:08, Greg KH wrote:
> On Sat, Aug 22, 2026 at 06:06:13PM +0200, Greg KH wrote:
>
> I don't think it can even build, given that there is no definition for
> machine_is_netwinder() anywhere in the tree that I can see.  So maybe
> many of these drivers can just be removed now.  Arnd, any ideas?

The machine_is_netwinder() function is autogenerated from
arch/arm/tools/mach-types, I would assume that the driver
still works as well as it ever has.

However, I have marked most arm32 board files including
this one as deprecated this merge window, and expect that
we can remove the platform and all associated drivers
for 7.4 or 7.5.

Please keep the driver for now, until we are sure the
platform removal goes ahead. I have already written
a patch to remove it, and plan to send it once the platform
removal is queued in the soc tree. I'll also take
care of nwflash, ds1620, mxc-rnga, and bh1770glc.

No need to fix any bugs in the driver though.

      Arnd

Re: nwbutton: lost wakeup in button_read() -- worth fixing, or is this driver done?

[old king]

Thanks for the clarification.
Understood -- I'll leave the driver as-is.

FAN YE

On Sun, Aug 23, 2026 at 2:59 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sat, Aug 22, 2026, at 18:08, Greg KH wrote:
> > On Sat, Aug 22, 2026 at 06:06:13PM +0200, Greg KH wrote:
> >
> > I don't think it can even build, given that there is no definition for
> > machine_is_netwinder() anywhere in the tree that I can see.  So maybe
> > many of these drivers can just be removed now.  Arnd, any ideas?
>
> The machine_is_netwinder() function is autogenerated from
> arch/arm/tools/mach-types, I would assume that the driver
> still works as well as it ever has.
>
> However, I have marked most arm32 board files including
> this one as deprecated this merge window, and expect that
> we can remove the platform and all associated drivers
> for 7.4 or 7.5.
>
> Please keep the driver for now, until we are sure the
> platform removal goes ahead. I have already written
> a patch to remove it, and plan to send it once the platform
> removal is queued in the soc tree. I'll also take
> care of nwflash, ds1620, mxc-rnga, and bh1770glc.
>
> No need to fix any bugs in the driver though.
>
>       Arnd