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

[Jason Ekstrand]

On Sun, Oct 5, 2014 at 2:23 PM, Karsten Otto <karsten.otto at posteo.de> wrote:

> Am 29.09.2014 um 06:31 schrieb Jason Ekstrand <jason at jlekstrand.net>:
>
> >
> > On Sep 28, 2014 11:49 AM, "Karsten Otto" <karsten.otto at posteo.de> wrote:
> > >
> > > From: Philip Withnall <philip at tecnocode.co.uk>
> > >
> > > This happens if the socket has been gracefully closed.
> > >
> > > [KAO: It prevents a potential infinite loop when using a different
> > > event handling mechanism than epoll, if said mechanism cannot
> > > distinguish EOF from regular read (e.g. select).]
> > > ---
> > >  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..85741cb 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 || (len < 0 && errno != EAGAIN)) {
> > >                         wl_client_destroy(client);
> > >                         return 1;
> >
> > It seems to me as if we should push this up the pipe to
> wl_connection_read so we fix it server-side too.
> >
> What do you mean? Calling wl_client_destroy within wl_client_read? Or
> making sure the connection is properly disposed of in both client and
> server code?
>
> Regarding the first, you would have to pass the wl_client reference to the
> read function, which would mix API layers unnecessarily. wl_client_read is
> basically just a convenient wrapper around recvmsg, so it should expose the
> same result semantics of >0 (OK), == 0 (EOF), and <0 (error), as it already
> does. It is up to the caller to handle the cases, if only to provide
> different log messages for error and EOF.
>
> Regarding the second, wl_connection_read is part of the wayland
> private/internal API, and only used in two places: wayland-client.c, which
> already handles EOF in an appropriate manner, and wayland-server.c, where
> the proposed patch handles the missing case as well. If anybody else needs
> to use wl_connection_read within the wayland libraries in the future, they
> are hopefully aware of the recv result semantics. If necessary, maybe add
> some documentation to wayland-private.h.
>

What I meant was to just make wl_connection_read return something
consistent (< 0 on EOF).  That said, if we're doing it this way in
wayland-client, then I'm fine with it.  I didn't actually go look.
--Jason Ekstrand


>
> > >                 }
> > > --
> > > 1.9.1
> > >
> > > _______________________________________________
> > > wayland-devel mailing list
> > > wayland-devel at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20141005/3977100e/attachment-0001.html>