linux-kernel - Re: [PATCH 3/4] input: evdev: Indicate buffer overrun with SYN

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: <20110325090210.GA5860@polaris.bitmath.org>
Date:	Fri, 25 Mar 2011 10:02:10 +0100
From:	"Henrik Rydberg" <rydberg@...omail.se>
To:	Jeff Brown <jeffbrown@...roid.com>
Cc:	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	Jeff Brown <jeffbrown@...gle.com>
Subject: Re: [PATCH 3/4] input: evdev: Indicate buffer overrun with
 SYN_DROPPED.

> @@ -57,14 +57,20 @@ static void evdev_pass_event(struct evdev_client *client,
>  {
>  	/*
>  	 * Interrupts are disabled, just acquire the lock.
> -	 * Make sure we don't leave with the client buffer
> -	 * "empty" by having client->head == client->tail.
> +	 * When the client buffer is full, replace the tail with SYN_DROPPED
> +	 * to let the client know that events were dropped.  Ensure that the
> +	 * head and tail never coincide so the buffer does not appear "empty".
>  	 */
>  	spin_lock(&client->buffer_lock);
> -	do {
> -		client->buffer[client->head++] = *event;
> -		client->head &= client->bufsize - 1;
> -	} while (client->head == client->tail);
> +	client->buffer[client->head++] = *event;
> +	client->head &= client->bufsize - 1;
> +	if (client->head == client->tail) {
> +		client->tail = (client->tail + 1) & (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;
> +	}
>  	spin_unlock(&client->buffer_lock);

My last comment was not right, the SYN_DROPPED is pushed ahead in the
buffer, sorry about that. However, this change does not shrink the
number of buffered elements in case of an overrun, which has been
discussed before as a possibly important feature of the current
code. I would be more comfortable prepending the head with a
SYN_DROPPED, like this:

if (client->head == client->tail) {
	struct input_event drop;

	drop.time = event->time;
	drop.type = EV_SYN;
	drop.code = SYN_DROPPED;
	drop.value = 0;
	client->buffer[client->head++] = drop;
	client->head &= client->bufsize - 1;

	client->buffer[client->head++] = *event;
	client->head &= client->bufsize - 1;
}

The main point is that if we end up having to drop an event, it is
likely we will have to drop the next one, too.

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/