[PATCH] Listening socket tweaks

[Kristian Høgsberg]

2011/2/7 Marty Jack <martyj19 at comcast.net>:
>
>
> 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?

No.  It will need root to open the input devices and control the tty.
The DRM modesetting ioctls are available to the DRM master, which is
the process the who opens the DRM device first.

> 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.

It depends on how Wayland is deployed.  If you're running the user
compositor directly on kms and evdev, then yes, we'll need priviledges
to open the evdev devices.  The other option is to run the user
compositor as a client of a priviledges "system compositor", which is
the one process that owns and controls kms and the input devices.  In
that case, the only priviledge the user compositor needs is a
connection to the system compositor.  There are a few options there,
my favorite idea is to just have the system compositor fork and exec
the user compositor and create a socket pair for the connection and
put the fd number in an environment variable.

Krisitan