Hi Dmitry,
On 12/8/23 11:58, Dmitry Torokhov wrote:
> Hi Vicki,
>
> On Wed, Dec 06, 2023 at 10:34:06PM -0800, Vicki Pfau wrote:
>> Any pending requests may be holding a mutex from its own subsystem, e.g.
>> evdev, while waiting to be able to claim the uinput device mutex.
>> However, unregistering the device may try to claim that mutex, leading
>> to a deadlock. To prevent this from happening, we need to temporarily
>> give up the lock before calling input_unregister_device.
>
> I do not think we can simply give up the lock, the whole thing with
> UI_DEV_DESTROY allowing reusing connection to create a new input device
> was a huge mistake because if you try to do UI_DEV_CREATE again on
> the same fd you'll end up reusing whatever is in udev instance,
> including the state and the mutex, and will make a huge mess.
Yeah, I was curious why this was possible in the first place. It seemed overcomplicated compared to just opening a new fd. I suppose that that makes more sense, though it's a bit involved for this.
>
> I think the only reasonable way forward is change the driver so that no
> ioctls are accepted after UI_DEV_DESTROY and then start untangling the
> locking issues (possibly by dropping the lock on destroy after setting
> the status - I think you will not observe the lockups you mention if
> your application will stop using UI_DEV_DESTROY and simply closes the
> fd).
This does sound like a reasonable way forward. Unfortunately, I don't have access to the uinput-side application code, but I have been trying to work with them to flatten out bugs in it. I can pass this suggestion along, though there is still a reproducible deadlock that could theoretically happen with other programs in the meantime (though the likelihood of it being hit without actively trying for it is low).
>
>>
>> Fixes: e8b95728f724 ("Input: uinput - avoid FF flush when destroying device")
>
> This is not the commit that introduced the problem, it has been there
> since forever.
My mistake. If I prepare a v2, which I may not, I'll drop the line.
>
> Thanks.
>
Vicki
