linux-kernel - Re: [PATCH] Input: fixed EVIOCGRAB iterative grab/release.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTikM3G5+KYXGe4hBL7LHg1-W0-dgN7jOGA2fbLvZ@mail.gmail.com>
Date:	Fri, 11 Feb 2011 09:55:53 -0800
From:	Terry Lambert <tlambert@...omium.org>
To:	Henrik Rydberg <rydberg@...omail.se>
Cc:	dmitry.torokhov@...il.com, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Input: fixed EVIOCGRAB iterative grab/release.

Hi Henrik,

I'm in the middle of cutting down my failure case right now, so I
should be able to provide a stand-alone test case shortly.  This is my
top priority at work right now.

And you are correct about the race.  This is about back-to-back
operations of a tool in a script, on a relatively slow machine with
obstinate hardware, and a grabby Xorg driver (it's the closed source
one, so I can't make it non-grabby).  So technically a stress test.

You noted the evdev->mutex; the idempotence of the call for the two
pointer clears will be preserved by the mutex, so no one is going to
get in under it while it's grabbed in the evdev_* layer, and not
grabbed in the input_*_device layer, and get unhappy.

So at worst it's a no-op change that scratches my particularly weird itch here.

PS: I supposed this use of the mutex is worthy of a comment in the
code?  Any guidance?

-- Terry

On Fri, Feb 11, 2011 at 2:04 AM, Henrik Rydberg <rydberg@...omail.se> wrote:
> Hi Terry,
>
>> Fixed order of calls in evdev_ungrab to allow iterative use of
>> code which grabs and releases input event devices.
>>
>> Signed-off-by: Terry Lambert <tlambert@...omium.org>
>> ---
>>  drivers/input/evdev.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
>> index c8471a2..0bac8da 100644
>> --- a/drivers/input/evdev.c
>> +++ b/drivers/input/evdev.c
>> @@ -160,9 +160,9 @@ static int evdev_ungrab(struct evdev *evdev, struct evdev_client *client)
>>       if (evdev->grab != client)
>>               return  -EINVAL;
>>
>> +     input_release_device(&evdev->handle);
>>       rcu_assign_pointer(evdev->grab, NULL);
>>       synchronize_rcu();
>> -     input_release_device(&evdev->handle);
>
> I imagine the current code could lead to a race situation if there
> were no other locks involved. However, evdev_ungrab() is always called
> under evdev->mutex. As Dmitry hinted, grabbing "usually works", so
> perhaps you could device a tiny program which reproduces the problem?
>
> Thanks,
> Henrik
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/