[PATCH] Listening socket tweaks

[Marty Jack]

On 02/07/2011 08:18 AM, Kristian Høgsberg wrote:
> On Mon, Feb 7, 2011 at 4:39 AM, Marty Jack <martyj19 at comcast.net> wrote:
>> With this patch, crashed compositors won't prevent a new one from starting, and unprivileged clients can connect.
> 
> Hi Marty,
> 
> The compositor is supposed to run as the user, not root, and the
> restrictive permissions on the unix socket is how we authorize access
> to the compositor.  The socket will be placed in $XDG_RUNTIME_DIR, see
> 
>   http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
> 
> If there's already a socket in there, we can't just unlink it, it
> could belong to a running compositor.
> 
> Kristian
> 
>> diff --git a/wayland/wayland-server.c b/wayland/wayland-server.c
>> index dece0d1..6b5f503 100644
>> --- a/wayland/wayland-server.c
>> +++ b/wayland/wayland-server.c
>> @@ -33,6 +33,7 @@
>>  #include <dlfcn.h>
>>  #include <assert.h>
>>  #include <sys/time.h>
>> +#include <sys/stat.h>
>>  #include <ffi.h>
>>
>>  #include "wayland-server.h"
>> @@ -675,10 +676,20 @@ wl_display_add_socket(struct wl_display *display, const char *name)
>>                             "%s/%s", runtime_dir, name) + 1;
>>        fprintf(stderr, "using socket %s\n", s->addr.sun_path);
>>
>> +       /* If the entry exists and is a socket, unlink it to prevent the bind from failing.
>> +        * It is probably a socket left over from a previous compositor that crashed. */
>> +       struct stat stat_buf;
>> +       if ((stat(s->addr.sun_path, &stat_buf) == 0)
>> +       && (S_ISSOCK(stat_buf.st_mode)))
>> +               unlink(s->addr.sun_path);
>> +
>>        size = offsetof (struct sockaddr_un, sun_path) + name_size;
>>        if (bind(s->fd, (struct sockaddr *) &s->addr, size) < 0)
>>                return -1;
>>
>> +       /* Set protections so unprivileged clients can connect. */
>> +       chmod(s->addr.sun_path, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
>> +
>>        if (listen(s->fd, 1) < 0)
>>                return -1;
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
> 

The DRM compositor will need CAP_SYS_ADMIN or root at least some of the time to do some of the ioctls it needs to do on the DRM device, yes?

So in the fullness of time it will need to raise and lower privileges.  I agree that it can be running as the user at the time it creates the socket if that is the way authorization is expected to work.