doubts concerning locking and signals in input_register_device()

doubts concerning locking and signals in input_register_device()

[Oliver Neukum]

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?

	Regards
		Oliver

Re: doubts concerning locking and signals in input_register_device()

[Dmitry Torokhov]

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

Re: doubts concerning locking and signals in input_register_device()

[Oliver Neukum]

On 24.06.22 02:46, Dmitry Torokhov wrote:
> Hi Oliver,

Hi,

> 
> 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?

timers, window size, ....

	Regards
		Oliver