Hi Pete,
On 2/28/07, Pete Zaitcev <[EMAIL PROTECTED]> wrote:
If a process is closing /dev/input/mice and an mouse disconnects simulta-
neously, the system is likely to oops. This usually happens when someone
hits <Alt><Ctrl>F1 or logs out from X, and flips a KVM while the system
is reacting.
I reproduced the issue by running this:
while true; do cat </dev/null >/dev/input/mice; done
This way, it oopses on 2nd or 3rd disconnect reliably. With the patch,
I can disconnect the mouse 20 times.
Signed-off-by: Pete Zaitcev <[EMAIL PROTECTED]>
---
Discussion
One of the race scenarios is related to the list of handles. The cat
calls mousedev_close -> mixdev_release, does list_for_each to walk for
all handles for a given handler. Iterations are longish while it does
input_close_device -> hidinput_close -> usbhid_close -> usb_kill_urb,
which sleeps briefly. Into this gap goes khubd and does hid_disconnect ->
hidinput_disconnect -> input_unregister_device. This corrupts the list
of handles which cat process is walking.
I was unable to devise a scheme to protect the stock h_list adequately,
so I implemented a private list of mousedev instances, which can be
protected correctly.
Dmitry, please consider getting rid of the list of handles entirely.
The other major user is drivers/char/keyboard.c.
I agree that handlers should not access h_list nad use their own
private lists instead. However input core still needs that list to
maintain its books.
Other than that, the patch is straightforward. It adds a static mutex
to guard common data structures. It has to be static because instances
of mousedev share common structures, such as the mousedev_table[].
This should be uncontroversial, but please let me know if I missed
something obvious.
I agree with the patch, unfortunately in lands squarely in the middle
of me restructuring the code (swiitch to struct device, proper
refcounting, etc) but I will try to adopt it.
--
Dmitry
