[PATCH 3/4] wayland-server: Abort if a read from a client gives 0 length

[Pekka Paalanen]

On Thu, 11 Sep 2014 21:42:26 +0200
Karsten Otto <karsten.otto at posteo.de> wrote:

> From: Philip Withnall <philip at tecnocode.co.uk>
> Date: Fri, 15 Feb 2013 12:57:05 +0000
> 
> This happens if the socket has been gracefully closed.
> 
> Signed-off-by: Philip Withnall <philip at tecnocode.co.uk>
> Signed-off-by: Karsten Otto <ottoka at posteo.de>
> ---
> src/wayland-server.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 674aeca..83e6f83 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -260,7 +260,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
> 	len = 0;
> 	if (mask & WL_EVENT_READABLE) {
> 		len = wl_connection_read(connection);
> -		if (len < 0 && errno != EAGAIN) {
> +		if (len <= 0 && errno != EAGAIN) {
> 			wl_client_destroy(client);
> 			return 1;
> 		}

I don't think this is correct. If wl_connection_read() returns zero, it
is because recvmsg() returned zero, which means errno has not been set.
If some earlier call caused errno to become EAGAIN, you still won't hit
the destroy path.

Why do we need this change here anyway? That would be good to explain
in the commit message. Does it fix a bug that happens only on BSD?
Or is it just avoid one more spin through the poll loop?

There is also a hypothetical problem here. If the fd polls readable,
but there isn't any data to be read, you would disconnect the client.
Arguably that should never happen, but should we trust that? The read
is explicitly non-blocking as well.


Thanks,
pq