Re: doubts concerning locking and signals in input_register_device()

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Oliver,

On Mon, Jun 20, 2022 at 02:48:01PM +0200, Oliver Neukum wrote:
> Hi,
> 
> I am looking at this code in input_register_device():
> 
>         error = mutex_lock_interruptible(&input_mutex);
>         if (error)
>                 goto err_device_del;
> 
> The problem is not the scope of the locking, but how the
> lock is taken.
> AFAICT this code will be called in the context of an
> ordinary task when you set a configuration through usbfs.
> usb_set_configuration() has to probe all interfaces
> of a device under its new configuration and does not
> handle -EINTR, which input_register_device() will
> return when a signal interrupts taking the mutex.
> 
> Hence there is a window for a race during which a
> mistimed harmless signal will cause a failure of probe()
> 
> Now, as this is fairly core code I'd rather ask what
> you think before I make the simple fix of taking
> the mutex with uninterruptible sleep. What is to be done?

The reason I believe we should be using interruptible or killable
versions of locking is to give a chance to bail (with Ctrl-C or kill or
whatever) in case the system is wedged for some reason.

Where would this mistimed harmless signal come from unless user actually
tries to interrupt module loading/driver binding/sysfs access?

Thanks.

-- 
Dmitry