Re: Fix locking in mousedev

[Dmitry Torokhov]

Hi Pete,

On 2/28/07, Pete Zaitcev <zaitcev@xxxxxxxxxx> 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 <zaitcev@xxxxxxxxxx>
>
> ---
>
> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/