On Thu, 17 Dec 2015 17:08:19 -0800 Bryce Harrington <[email protected]> wrote:
> This adds functionality to allow system-level control over handing out > file descriptors for sockets, to allow tighter security when running a > Wayland compositor under a Wayland session server. Hi Bryce, this is looking good. Commit message could also mention, that this new API also allows writing socket activated Wayland servers. > Signed-off-by: Bryce Harrington <[email protected]> > Cc: Sung-Jin Park <[email protected]> > Cc: Sangjin Lee <[email protected]> > > --- > v2: > + Drop tab corrections > + Add patch to move if statement into assert > > v3: > + Removed wl_os_socket_check_cloexec > + Removed wl_display_add_socket_fd_auto > + Replaced _wl_display_add_socket > + Rewrote wl_display_add_socket_fd > > v4: > + Rewrote wl_display_add_socket_fd > + Remove everything except just storing the fd > + Document that we're assuming the caller sets up the fd properly > > src/wayland-server-core.h | 3 +++ > src/wayland-server.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h > index 85b4b9e..1700cd3 100644 > --- a/src/wayland-server-core.h > +++ b/src/wayland-server-core.h > @@ -131,6 +131,9 @@ wl_display_add_socket(struct wl_display *display, const > char *name); > const char * > wl_display_add_socket_auto(struct wl_display *display); > > +int > +wl_display_add_socket_fd(struct wl_display *display, int sock_fd); > + > void > wl_display_terminate(struct wl_display *display); > > diff --git a/src/wayland-server.c b/src/wayland-server.c > index 1364d5d..a4fcc96 100644 > --- a/src/wayland-server.c > +++ b/src/wayland-server.c > @@ -1198,6 +1198,49 @@ wl_display_add_socket_auto(struct wl_display *display) > return NULL; > } > > +/** Add a socket with an existing fd to Wayland display for the clients to > connect. > + * > + * \param display Wayland display to which the socket should be added. > + * \param name Name of the Unix socket. There is no 'name' anymore, but sock_fd is. > + * \return 0 if success. -1 if failed. > + * > + * The existing socket fd must already be created, opened, and locked. > + * The fd must be properly set to CLOEXEC and bound to a socket file > + * with both bind() and listen() already called. Yeah, I think that is the complete list of requirements. Very good. > + * > + * \memberof wl_display > + */ > +WL_EXPORT int > +wl_display_add_socket_fd(struct wl_display *display, int sock_fd) > +{ > + struct wl_socket *s; > + struct stat buf; > + > + /* Require a valid fd or fail */ > + if (sock_fd < 0 || fstat(sock_fd, &buf) < 0 || !S_ISSOCK(buf.st_mode)) { > + return -1; > + } > + > + s = wl_socket_alloc(); > + if (s == NULL) > + return -1; > + > + /* Reuse the existing fd */ > + s->fd = sock_fd; > + > + s->source = wl_event_loop_add_fd(display->loop, s->fd, > + WL_EVENT_READABLE, > + socket_data, display); > + if (s->source == NULL) { > + wl_log("failed to establish event source\n"); > + return -1; > + } > + > + wl_list_insert(display->socket_list.prev, &s->link); > + > + return 0; > +} The implementation looks good. A good decision to polish this first so you don't have to change the tests twice more if there were anything wrong here. > + > /** Add a socket to Wayland display for the clients to connect. > * > * \param display Wayland display to which the socket should be added. If you fix the doc comment, and regardless if you add to the commit message or not: Reviewed-by: Pekka Paalanen <[email protected]> There was some good material in your earlier series, I hope to see those going forward too. :-) Thanks, pq
pgpsku0WR3_Ik.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
