[PATCH 3/8] server: add *BSD credentials support.
Pekka Paalanen
ppaalanen at gmail.com
Fri Feb 8 14:26:13 UTC 2019
On Fri, 8 Feb 2019 15:13:36 +0200
Leonid Bobrov <mazocomp at disroot.org> wrote:
> Signed-off-by: Leonid Bobrov <mazocomp at disroot.org>
> ---
> configure.ac | 3 ++
> src/wayland-server.c | 46 ++++++++++++++++++++++
> src/wayland-shm.c | 92 +++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 140 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 912330e..d106a41 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -65,6 +65,9 @@ AC_SUBST(GCC_CFLAGS)
> AC_CHECK_HEADERS([sys/prctl.h])
> AC_CHECK_FUNCS([accept4 mkostemp posix_fallocate prctl])
>
> +# Credential support on BSD
> +AC_CHECK_HEADERS([sys/ucred.h])
> +
> # Replacement for /proc on BSD
> AC_CHECK_HEADERS([kvm.h])
> SAVE_LIBS="$LIBS"
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 19f6a76..f4cdbf3 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -25,6 +25,13 @@
>
> #define _GNU_SOURCE
>
> +#include "../config.h"
Hi Leonid,
oh, we're missing config.h in lots of places. Adding that everywhere
should probably be a patch in front of the series. Usually that's just
#include "config.h"
with the build system setting the search paths appropriately.
> +
> +#ifdef HAVE_SYS_UCRED_H
> +#include <sys/types.h>
> +#include <sys/ucred.h>
> +#endif
> +
> #include <stdlib.h>
> #include <stdint.h>
> #include <stddef.h>
> @@ -77,7 +84,13 @@ struct wl_client {
> struct wl_list link;
> struct wl_map objects;
> struct wl_priv_signal destroy_signal;
> +#ifdef HAVE_SYS_UCRED_H
> + /* BSD */
> + struct xucred xucred;
> +#else
> + /* Linux */
> struct ucred ucred;
> +#endif
Maybe the above should be replaced with just pid, uid and gid fields,
to avoid the #ifdefs here and elsewhere.
> int error;
> struct wl_priv_signal resource_created_signal;
> };
> @@ -312,7 +325,11 @@ wl_resource_post_error(struct wl_resource *resource,
> static void
> destroy_client_with_error(struct wl_client *client, const char *reason)
> {
> +#ifdef HAVE_SYS_UCRED_H
> + wl_log("%s (uid %u)\n", reason, client->xucred.cr_uid);
Printing uid will be as useful as printing pid 0 always, since the user
is practically always the same. I'd drop this change.
> +#else
> wl_log("%s (pid %u)\n", reason, client->ucred.pid);
> +#endif
> wl_client_destroy(client);
> }
>
> @@ -526,10 +543,29 @@ wl_client_create(struct wl_display *display, int fd)
> if (!client->source)
> goto err_client;
>
> +#ifndef SO_PEERCRED
> +/* FreeBSD */
> +# define SO_PEERCRED LOCAL_PEERCRED
> +#endif
This should go to wayland-os.h.
> +
> +#ifdef HAVE_SYS_UCRED_H
> + /* BSD */
> + len = sizeof client->xucred;
> + if (getsockopt(fd, SOL_SOCKET, SO_PEERCRED,
> + &client->xucred, &len) < 0
> +# ifdef XUCRED_VERSION
> + /* FreeBSD */
> + || client->xucred.cr_version != XUCRED_VERSION
> +# endif
> + )
> + goto err_source;
> +#else
> + /* Linux */
> len = sizeof client->ucred;
> if (getsockopt(fd, SOL_SOCKET, SO_PEERCRED,
> &client->ucred, &len) < 0)
> goto err_source;
> +#endif
Maybe wayland-os.c should have a wrapper for getsockopt(PEERCRED),
these #ifdefs are not nice.
>
> client->connection = wl_connection_create(fd);
> if (client->connection == NULL)
> @@ -583,12 +619,22 @@ WL_EXPORT void
> wl_client_get_credentials(struct wl_client *client,
> pid_t *pid, uid_t *uid, gid_t *gid)
> {
> +#ifdef HAVE_SYS_UCRED_H
> + /* BSD */
> + *pid = 0; /* FIXME: pid is not defined on BSD */
> + if (uid)
> + *uid = client->xucred.cr_uid;
> + if (gid)
> + *gid = client->xucred.cr_gid;
> +#else
> + /* Linux */
> if (pid)
> *pid = client->ucred.pid;
> if (uid)
> *uid = client->ucred.uid;
> if (gid)
> *gid = client->ucred.gid;
> +#endif
> }
>
> /** Get the file descriptor for the client
> diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> index 4191231..dbaf464 100644
> --- a/src/wayland-shm.c
> +++ b/src/wayland-shm.c
> @@ -30,6 +30,8 @@
>
> #define _GNU_SOURCE
>
> +#include "../config.h"
> +
> #include <stdbool.h>
> #include <stdio.h>
> #include <stdlib.h>
> @@ -59,6 +61,9 @@ struct wl_shm_pool {
> char *data;
> int32_t size;
> int32_t new_size;
> +#ifdef HAVE_SYS_UCRED_H
> + int fd;
This field could be present unconditionally, and on Linux it would get
initialized to -1 and nothing more. Then you don't have to #ifdef the
close() of it either.
> +#endif
> };
>
> struct wl_shm_buffer {
> @@ -76,15 +81,91 @@ struct wl_shm_sigbus_data {
> int fallback_mapping_used;
> };
>
> +#ifdef HAVE_MREMAP
> +static void *
> +mremap_compat_maymove(void *old_address, size_t old_size, size_t new_size,
> + int old_prot, int old_flags, int old_fd)
> +{
> + return mremap(old_address, old_size, new_size, MREMAP_MAYMOVE);
> +}
> +#else
> +static void *
> +mremap_compat_maymove(void *old_address, size_t old_size, size_t new_size,
> + int old_prot, int old_flags, int old_fd)
> +{
> + /* FreeBSD doesn't support mremap() yet, so we have to emulate it.
> + * This assumes MREMAP_MAYMOVE is the only flag in use. */
> + if (new_size == old_size) {
> + return old_address;
> + } else if (new_size < old_size) {
> + /* Shrinking: munmap() the spare region. */
> + munmap(old_address + old_size, new_size - old_size);
> + return old_address;
> + } else {
> + void *ret;
> +
> + /* Growing. Try and mmap() the extra region at the end of
> + * our existing allocation. If that gets mapped in the
> + * wrong place, fall back to mmap()ing an entirely new
> + * region of new_size and copying the data across. */
> + ret = mmap(old_address + old_size, new_size - old_size,
> + old_prot, old_flags, old_fd, 0);
> +
> +/* FIXME TODO: msync() before munmap()? */
> + if (ret == MAP_FAILED) {
> + /* Total failure! */
> + return ret;
> + } else if (ret == old_address + old_size) {
> + /* Success. */
> + return old_address;
> + } else if (ret != old_address + old_size) {
> + /* Partial failure. Fall back to mapping an
> + * entirely new region. Unmap the region we
> + * just mapped first. */
> + munmap(ret, new_size - old_size);
> +
> + /* Map an entirely new region. */
> + ret = mmap(NULL, new_size,
> + old_prot, old_flags, old_fd, 0);
> + if (ret == MAP_FAILED) {
> + /* Total failure! */
> + return ret;
> + }
> +
> + /* Copy the old data across. Implicit assumption
> + * that the old and new regions don't overlap. */
> + memcpy(ret, old_address, old_size);
> +
> + /* Unmap the old region. */
> + munmap(old_address, old_size);
> +
> + return ret;
> + }
> + }
> +
> + /* Unreachable. */
> + return MAP_FAILED;
> +}
> +#endif
mremap_compat_maymove() should be in wayland-os.c. What does this has
to do with credentials support, shouldn't this be a separate patch?
> +
> static void
> shm_pool_finish_resize(struct wl_shm_pool *pool)
> {
> void *data;
> +#ifdef HAVE_SYS_UCRED_H
Shouldn't all these be HAVE_MREMAP instead?
> + int fd = -1;
> +#endif
>
> if (pool->size == pool->new_size)
> return;
>
> +#ifdef HAVE_SYS_UCRED_H
> + fd = pool->fd;
> + data = mremap_compat_maymove(pool->data, pool->size, pool->new_size,
> + PROT_READ|PROT_WRITE, MAP_SHARED, fd);
> +#else
> data = mremap(pool->data, pool->size, pool->new_size, MREMAP_MAYMOVE);
> +#endif
This is a redundant fallback. mremap_compat_maymove() works also for
Linux, so just call that unconditionally.
> if (data == MAP_FAILED) {
> wl_resource_post_error(pool->resource,
> WL_SHM_ERROR_INVALID_FD,
> @@ -110,6 +191,10 @@ shm_pool_unref(struct wl_shm_pool *pool, bool external)
> if (pool->internal_refcount + pool->external_refcount)
> return;
>
> +#ifdef HAVE_SYS_UCRED_H
> + close(pool->fd);
> +#endif
> +
> munmap(pool->data, pool->size);
> free(pool);
> }
> @@ -284,7 +369,13 @@ shm_create_pool(struct wl_client *client, struct wl_resource *resource,
> "failed mmap fd %d: %m", fd);
> goto err_free;
> }
> +#ifdef HAVE_SYS_UCRED_H
> + /* We need to keep the FD around on FreeBSD so we can implement
> + * mremap(). See: mremap_compat_maymove(). */
> + pool->fd = fd;
> +#else
> close(fd);
> +#endif
Yeah, this #ifdef is necessary it seems.
>
> pool->resource =
> wl_resource_create(client, &wl_shm_pool_interface, 1, id);
> @@ -364,7 +455,6 @@ wl_shm_buffer_get_stride(struct wl_shm_buffer *buffer)
> return buffer->stride;
> }
>
> -
> /** Get a pointer to the memory for the SHM buffer
> *
> * \param buffer The buffer object
Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20190208/1fc22f35/attachment.sig>
More information about the wayland-devel
mailing list