While investigating a report of a process hanging on submitting ff data to a uinput-derived evdev handle I uncovered several issues regarding cross-thread concurrency. The first fix is simply making waiting on the completion object interruptible. Without this, the submitting process cannot be interrupted, meaning it has to either wait for the uinput-controlling process to read the data, or the timeout being reached. While this is the usual flow, it being uninterruptible means that if the uinput-controlling process is misbehaving, the submitting process cannot be killed, suspended, or otherwise interrupted until the timeout is reached, which could take an annoyingly long time for users. The second fix is probably more controversial, and I'm unsure if it's really the best way to solve this issue. Namely, there exists a small, but reproducible window where closing a uinput device on the uinput side and uploading ff data via an evdev handle in a separate process will lead to a deadlock: the uinput ioctl will claim the mutex, flush requests, then try to close the input device, which then tries to claim the evdev mutex. However, when uploading the ff data, the evdev mutex will be claimed, try to claim the uinput mutex, and hang indefinitely, leading to a deadlock. Since it can never claim the uinput mutex, it doesn't notice that it should exit early, but since it can't get the mutex at all, it can't release the evdev mutex. My approach to solving this involves temporarily releasing the mutex after flushing requests, allowing the upload to claim the mutex, then closing the input device without the mutex being held, and finally reclaim the mutex to rebalance the mutex_unlock later on. I spent quite a while investigating other approaches while trying to come up with the least hacky and simplest way to fix this. However, a proper fix might be more involved and have to touch other subsystems, namely evdev, in which case I would defer to Dmitry for a better fix, as he's a lot more familiar with these subsystems. I also suspect that there's a race condition with uinput_dev_event, as most call sites are protected by the uinput device mutex, but not all of them. Namely, it can be called via the input device's event function pointer, which has no idea that the uinput mutex exists. However, I haven't demonstrated that there is actually an issue here, so I haven't attempted to fix it. Vicki Pfau (2): Input: uinput - Allow uinput_request_submit wait interrupting Input: uinput - Release mutex while unregistering input device drivers/input/misc/uinput.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) -- 2.43.0
