insunficient locking in input.c (?)

[Pavel Machek <pavel@suse.cz>]

Hi!

It seems to me some more locking is needed:

1) input_event() seems to be called from both keyboard interupt and
from timer. That makes it pretty nasty beast.

It does:

...
                list_for_each_entry(handle, &dev->h_list, d_node)
                        if (handle->open)
                                handle->handler->event(handle, type, code, value);
...

while input_unregister_handler could be running (on other CPU?)

void input_unregister_handler(struct input_handler *handler)
{
        struct list_head * node, * next;

        list_for_each_safe(node, next, &handler->h_list) {
                struct input_handle * handle = to_handle_h(node);
                list_del_init(&handle->h_node);
                list_del_init(&handle->d_node);
                handler->disconnect(handle);
        }

I guess that some locking around these lists is needed.

2) input_event() is called from both keyboard interupt and from
timer. That makes it behave pretty badly w.r.t. low-level handlers. I
think that you can have one low-level handler running two times
concurrently.

There's no locking preventing that. AFAICS autorepeat timer can tick
once more after key is released, which is normally not a problem, but
if key is pressed while that, it looks to me like there can be normal
key down and autorepeat entering input_event() concurrently, which
then calls low-level handler (for example kbd_keycode)
concurrently. kbd_keycode() certainly is not written to handle _that_.

								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]