| 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
| ||
|
Message-ID: <CADYu30_uxvqe0c7tGjzvhBtQuYz=cvnb71TztCCxQER6LwhwAg@mail.gmail.com>
Date: Sat, 5 Nov 2016 01:27:37 +0530
From: Aniroop Mathur <aniroop.mathur@...il.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>,
"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc: Aniroop Mathur <a.mathur@...sung.com>
Subject: Re: [PATCH] [v9]Input: evdev: fix bug of dropping valid packet after
syn_dropped event
Hello Mr. Torokhov,
As you can see that this patch is pending for a long time,
I request you to please review it at the earliest possible.
Thanks,
Aniroop Mathur
On Thu, Oct 13, 2016 at 12:05 AM, Aniroop Mathur
<aniroop.mathur@...il.com> wrote:
> Hello Mr. Torokhov,
>
> Hope you are doing great!
>
> Could you please help to update about below version of the patch?
>
> --
> Copying text about last two problems in v8:
>
> Problem 1: Handle EVIOCG[type] for queue empty case
> --> Done
> Queue empty condition needs to be added before calling evdev_queue_syn_dropped.
> Since there is a rare chance that new packet gets inserted between buffer_lock
> mutex unlocking and copying events to user space, so I will check whether queue
> is empty or not before __evdev_flush_queue and then use it before invoking
> evdev_queue_syn_dropped.
> And if queue is not empty then there is no problem as after flushing some
> events we always have atleast one SYN_REPORT event left in queue.
>
> Problem 2: We try to pass full packets to clients, but there is no guarantee.
> --> Done
> From my earlier patch discussion regarding dropping syn_report in between a
> single packet it was deduced that since no driver is currently using that code
> so there is no impact. Hence for this problem too, we are good to go.~
> (Although from code reading/learning perspective, I would still suggest to not
> have code of forceful sun_report event addition even if no driver using that
> code :-) )
>
> Have a good day!
>
> Best Regards,
> Aniroop Mathur
>
>
>> On Wed, Oct 5, 2016 at 12:42 AM, Aniroop Mathur <a.mathur@...sung.com> wrote:
>>> If last event dropped in the old queue was EV_SYN/SYN_REPORT, then lets
>>> generate EV_SYN/SYN_REPORT immediately after queing EV_SYN/SYN_DROPPED
>>> so that clients would not ignore next valid full packet events.
>>>
>>> Signed-off-by: Aniroop Mathur <a.mathur@...sung.com>
>>>
>>> Difference from v8:
>>> Added check for handling EVIOCG[type] ioctl for queue empty case
>>> ---
>>> drivers/input/evdev.c | 52 ++++++++++++++++++++++++++++++++++++++-------------
>>> 1 file changed, 39 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
>>> index e9ae3d5..69407ff 100644
>>> --- a/drivers/input/evdev.c
>>> +++ b/drivers/input/evdev.c
>>> @@ -156,7 +156,12 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
>>> static void __evdev_queue_syn_dropped(struct evdev_client *client)
>>> {
>>> struct input_event ev;
>>> + struct input_event *last_ev;
>>> ktime_t time;
>>> + unsigned int mask = client->bufsize - 1;
>>> +
>>> + /* capture last event stored in the buffer */
>>> + last_ev = &client->buffer[(client->head - 1) & mask];
>>>
>>> time = client->clk_type == EV_CLK_REAL ?
>>> ktime_get_real() :
>>> @@ -170,13 +175,28 @@ static void __evdev_queue_syn_dropped(struct evdev_client *client)
>>> ev.value = 0;
>>>
>>> client->buffer[client->head++] = ev;
>>> - client->head &= client->bufsize - 1;
>>> + client->head &= mask;
>>>
>>> if (unlikely(client->head == client->tail)) {
>>> /* drop queue but keep our SYN_DROPPED event */
>>> - client->tail = (client->head - 1) & (client->bufsize - 1);
>>> + client->tail = (client->head - 1) & mask;
>>> client->packet_head = client->tail;
>>> }
>>> +
>>> + /*
>>> + * If last packet was completely stored, then queue SYN_REPORT
>>> + * so that clients would not ignore next valid full packet
>>> + */
>>> + if (last_ev->type == EV_SYN && last_ev->code == SYN_REPORT) {
>>> + last_ev->time = ev.time;
>>> + client->buffer[client->head++] = *last_ev;
>>> + client->head &= mask;
>>> + client->packet_head = client->head;
>>> +
>>> + /* drop queue but keep our SYN_DROPPED & SYN_REPORT event */
>>> + if (unlikely(client->head == client->tail))
>>> + client->tail = (client->head - 2) & mask;
>>> + }
>>> }
>>>
>>> static void evdev_queue_syn_dropped(struct evdev_client *client)
>>> @@ -218,7 +238,7 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
>>> spin_lock_irqsave(&client->buffer_lock, flags);
>>>
>>> if (client->head != client->tail) {
>>> - client->packet_head = client->head = client->tail;
>>> + client->packet_head = client->tail = client->head;
>>> __evdev_queue_syn_dropped(client);
>>> }
>>>
>>> @@ -231,22 +251,24 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
>>> static void __pass_event(struct evdev_client *client,
>>> const struct input_event *event)
>>> {
>>> + unsigned int mask = client->bufsize - 1;
>>> +
>>> client->buffer[client->head++] = *event;
>>> - client->head &= client->bufsize - 1;
>>> + client->head &= mask;
>>>
>>> if (unlikely(client->head == client->tail)) {
>>> /*
>>> * This effectively "drops" all unconsumed events, leaving
>>> - * EV_SYN/SYN_DROPPED plus the newest event in the queue.
>>> + * EV_SYN/SYN_DROPPED, EV_SYN/SYN_REPORT (if required) and
>>> + * newest event in the queue.
>>> */
>>> - client->tail = (client->head - 2) & (client->bufsize - 1);
>>> -
>>> - client->buffer[client->tail].time = event->time;
>>> - client->buffer[client->tail].type = EV_SYN;
>>> - client->buffer[client->tail].code = SYN_DROPPED;
>>> - client->buffer[client->tail].value = 0;
>>> + client->head = (client->head - 1) & mask;
>>> + client->packet_head = client->tail = client->head;
>>> + __evdev_queue_syn_dropped(client);
>>>
>>> - client->packet_head = client->tail;
>>> + client->buffer[client->head++] = *event;
>>> + client->head &= mask;
>>> + /* No need to check for buffer overflow as it just occurred */
>>> }
>>>
>>> if (event->type == EV_SYN && event->code == SYN_REPORT) {
>>> @@ -920,6 +942,7 @@ static int evdev_handle_get_val(struct evdev_client *client,
>>> int ret;
>>> unsigned long *mem;
>>> size_t len;
>>> + bool is_queue_empty;
>>>
>>> len = BITS_TO_LONGS(maxbit) * sizeof(unsigned long);
>>> mem = kmalloc(len, GFP_KERNEL);
>>> @@ -933,12 +956,15 @@ static int evdev_handle_get_val(struct evdev_client *client,
>>>
>>> spin_unlock(&dev->event_lock);
>>>
>>> + if (client->head == client->tail)
>>> + is_queue_empty = true;
>>> +
>>> __evdev_flush_queue(client, type);
>>>
>>> spin_unlock_irq(&client->buffer_lock);
>>>
>>> ret = bits_to_user(mem, maxbit, maxlen, p, compat);
>>> - if (ret < 0)
>>> + if (ret < 0 && !is_queue_empty)
>>> evdev_queue_syn_dropped(client);
>>>
>>> kfree(mem);
>>> --
>>> 2.6.2
>>>
Powered by blists - more mailing lists