linux-kernel - Re: [PATCH] Input: serio_raw - ensure we don't block in non-blocking read

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CANq1E4TOG0wgzJ43px74204k9KgRWpDxRPJhC45BiAERTcwsxg@mail.gmail.com>
Date:	Fri, 13 Apr 2012 18:50:22 +0200
From:	David Herrmann <dh.herrmann@...glemail.com>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	linux-input@...r.kernel.org,
	Wanlong Gao <gaowanlong@...fujitsu.com>,
	Che-Liang Chiou <clchiou@...omium.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Input: serio_raw - ensure we don't block in non-blocking read

Hi Dmitry

On Wed, Apr 11, 2012 at 9:23 AM, Dmitry Torokhov
<dmitry.torokhov@...il.com> wrote:
> On Tue, Apr 03, 2012 at 01:24:33PM -0700, Dmitry Torokhov wrote:
>> Avoid calling wait_event_interruptible() if client requested non-blocking
>> read, since it is not guaranteed that another thread will not consume
>> event after we checked if serio_raw->head != serio_raw->tail.
>>
>> Also ensure we do not return 0 but keep waiting instead in blocking case,
>> when another thread steals "our" byte.
>>
>> Signed-off-by: Dmitry Torokhov <dtor@...l.ru>
>
> Anyone? It would be nice if someone could review my patches as well,
> otherwise we may end up with breakages...

I cannot test the driver as I don't have the devices, but I reviewed
it and it looks fine.

> Thanks.

Regards
David

>> ---
>>  drivers/input/serio/serio_raw.c |   43 ++++++++++++++++++++++----------------
>>  1 files changed, 25 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/input/serio/serio_raw.c b/drivers/input/serio/serio_raw.c
>> index 4494233..094d6fb 100644
>> --- a/drivers/input/serio/serio_raw.c
>> +++ b/drivers/input/serio/serio_raw.c
>> @@ -165,31 +165,38 @@ static ssize_t serio_raw_read(struct file *file, char __user *buffer,
>>       struct serio_raw *serio_raw = client->serio_raw;
>>       char uninitialized_var(c);
>>       ssize_t read = 0;
>> -     int retval;
>> +     int error = 0;
>>
>> -     if (serio_raw->dead)
>> -             return -ENODEV;
>> +     do {
>> +             if (serio_raw->dead)
>> +                     return -ENODEV;
>>
>> -     if (serio_raw->head == serio_raw->tail && (file->f_flags & O_NONBLOCK))
>> -             return -EAGAIN;
>> +             if (serio_raw->head == serio_raw->tail &&
>> +                 (file->f_flags & O_NONBLOCK))
>> +                     return -EAGAIN;
>>
>> -     retval = wait_event_interruptible(serio_raw->wait,
>> -                     serio_raw->head != serio_raw->tail || serio_raw->dead);
>> -     if (retval)
>> -             return retval;
>> +             if (count == 0)
>> +                     break;
>>
>> -     if (serio_raw->dead)
>> -             return -ENODEV;
>> +             while (read < count && serio_raw_fetch_byte(serio_raw, &c)) {
>> +                     if (put_user(c, buffer++)) {
>> +                             error = -EFAULT;
>> +                             goto out;
>> +                     }
>> +                     read++;
>> +             }
>>
>> -     while (read < count && serio_raw_fetch_byte(serio_raw, &c)) {
>> -             if (put_user(c, buffer++)) {
>> -                     retval = -EFAULT;
>> +             if (read)
>>                       break;
>> -             }
>> -             read++;
>> -     }
>>
>> -     return read ?: retval;
>> +             if (!(file->f_flags & O_NONBLOCK))
>> +                     error = wait_event_interruptible(serio_raw->wait,
>> +                                     serio_raw->head != serio_raw->tail ||
>> +                                     serio_raw->dead);
>> +     } while (!error);
>> +
>> +out:
>> +     return read ?: error;
>>  }
>>
>>  static ssize_t serio_raw_write(struct file *file, const char __user *buffer,
>> --
>> 1.7.7.6
>>
>
> --
> Dmitry
--
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/