~kennylevinsen/seatd-devel

seatd: handle client_create failure v1 PROPOSED

Simon Ser: 3
 seatd: handle client_create failure
 seatd: refuse to compile with missing get_peer impl
 wip: Add OpenBSD support

 8 files changed, 58 insertions(+), 7 deletions(-)
Neat! I suppose some of the wscons-related changes could also be 
applied to NetBSD. Drew linked swc as a source of inspiration in 
https://todo.sr.ht/~kennylevinsen/seatd/11.

Could you add an OpenBSD build manifest when you get around to it?

On Sat, Oct 23 2021 at 02:27:41 PM +0000, Simon Ser 
<contact@emersion.fr> wrote:
Next
I need to find myself a better email client.
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~kennylevinsen/seatd-devel/patches/25897/mbox | git am -3
Learn more about email & git

[PATCH 1/3] seatd: handle client_create failure Export this patch

Failure to create the client causes a null pointer dereference.
---
 seatd/server.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/seatd/server.c b/seatd/server.c
index 0a08f50bf800..73e6a2bc2463 100644
--- a/seatd/server.c
+++ b/seatd/server.c
@@ -129,6 +129,12 @@ int server_add_client(struct server *server, int fd) {
	}

	struct client *client = client_create(server, fd);
	if (client == NULL) {
		log_errorf("Could not create client: %s", strerror(errno));
		close(fd);
		return -1;
	}

	client->event_source =
		poller_add_fd(&server->poller, fd, EVENT_READABLE, client_handle_connection, client);
	if (client->event_source == NULL) {
-- 
2.33.1

[PATCH 2/3] seatd: refuse to compile with missing get_peer impl Export this patch

Instead of using a stub, error out when get_peer isn't implemented
for the target platform. client_create relies on it.
---
 seatd/client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/seatd/client.c b/seatd/client.c
index 220c5d3ef3dd..53d666369413 100644
--- a/seatd/client.c
+++ b/seatd/client.c
@@ -49,7 +49,7 @@ static int get_peer(int fd, pid_t *pid, uid_t *uid, gid_t *gid) {
	*gid = cred.cr_ngroups > 0 ? cred.cr_groups[0] : (gid_t)-1;
	return 0;
#else
	return -1;
#error Unsupported platform
#endif
}

-- 
2.33.1

[PATCH 3/3] wip: Add OpenBSD support Export this patch

With this the tests all run fine, but I wasn't able to get the
example running yet because of missing DRM virtio support. seatd
errors out with:

    [seatd/seat.c:277] Could not open file: Device not configured

likely because none of the DRM kernel drivers are loaded.

The evdev stubbing is unpleasant, need to investigate what's needed
to add proper wscons support. Also OpenBSD doesn't have SIGRTMIN.
---
 common/drm.c      |  6 ++++++
 common/terminal.c | 16 +++++++++++++++-
 meson.build       | 16 +++++++++++++---
 seatd/client.c    |  3 +++
 seatd/seat.c      | 12 ++++++++++--
 tests/poller.c    |  4 ++++
 6 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/common/drm.c b/common/drm.c
index 9591dc06bc9d..44f086011c63 100644
--- a/common/drm.c
+++ b/common/drm.c
@@ -40,6 +40,12 @@ int path_is_drm(const char *path) {
	static const int prefixlen = STRLEN(prefix);
	return strncmp(prefix, path, prefixlen) == 0;
}
#elif defined(__OpenBSD__)
int path_is_drm(const char *path) {
	static const char prefix[] = "/dev/drm";
	static const int prefixlen = STRLEN(prefix);
	return strncmp(prefix, path, prefixlen) == 0;
}
#else
#error Unsupported platform
#endif
diff --git a/common/terminal.c b/common/terminal.c
index 0c3466fe572f..cecfa2f15091 100644
--- a/common/terminal.c
+++ b/common/terminal.c
@@ -21,6 +21,12 @@
#define K_ENABLE  K_XLATE
#define K_DISABLE K_RAW
#define FRSIG     SIGIO
#elif defined(__OpenBSD__)
#define K_ENABLE  K_XLATE
#define K_DISABLE K_RAW
#define FRSIG     SIGIO
#include <sys/types.h>
#include <dev/wscons/wsdisplay_usl_io.h>
#else
#error Unsupported platform
#endif
@@ -134,6 +140,14 @@ static int get_tty_path(int tty, char path[static TTYPATHLEN]) {
	}
	return 0;
}
#elif defined(__OpenBSD__)
static int get_tty_path(int tty, char path[static TTYPATHLEN]) {
	assert(tty >= 0);
	if (snprintf(path, TTYPATHLEN, "/dev/ttyC%d", tty) == -1) {
		return -1;
	}
	return 0;
}
#else
#error Unsupported platform
#endif
@@ -162,7 +176,7 @@ int terminal_current_vt(int fd) {
		return -1;
	}
	return st.v_active;
#elif defined(__FreeBSD__)
#elif defined(__FreeBSD__) || defined(__OpenBSD__)
	int vt;
	int res = ioctl(fd, VT_GETACTIVE, &vt);
	close(fd);
diff --git a/meson.build b/meson.build
index d8fd25dfe645..44c47e0cb68c 100644
--- a/meson.build
+++ b/meson.build
@@ -41,7 +41,8 @@ add_project_arguments(cc.get_supported_arguments(
		'-Wvla',
		'-Wl,--exclude-libs=ALL',
		'-D_XOPEN_SOURCE=700',
		'-D__BSD_VISIBLE',
		'-D_BSD_SOURCE=1',
		'-D__BSD_VISIBLE=1',
		'-DSEATD_VERSION="@0@"'.format(meson.project_version()),
		'-DSEATD_DEFAULTPATH="@0@"'.format(defaultpath),
		'-DSEATD_INSTALLPATH="@0@"'.format(seatdpath),
@@ -98,7 +99,6 @@ server_files = [
	'common/linked_list.c',
	'common/terminal.c',
	'common/connection.c',
	'common/evdev.c',
	'common/drm.c',
	'seatd/poller.c',
	'seatd/seat.c',
@@ -106,6 +106,10 @@ server_files = [
	'seatd/server.c',
]

if host_machine.system() != 'openbsd'
	server_files += 'common/evdev.c'
endif

with_seatd = get_option('libseat-seatd') == 'enabled'
with_builtin = get_option('libseat-builtin') == 'enabled'
with_server = get_option('server') == 'enabled'
@@ -148,7 +152,13 @@ if get_option('libseat-logind') != 'disabled'
endif

# needed for cross-compilation
realtime = meson.get_compiler('c').find_library('rt')
# OpenBSD has no librt
if host_machine.system() != 'openbsd'
  realtime = meson.get_compiler('c').find_library('rt')
else
  realtime = dependency('', required: false)
endif

private_deps += realtime

if with_builtin
diff --git a/seatd/client.c b/seatd/client.c
index 53d666369413..c19b3f7775f5 100644
--- a/seatd/client.c
+++ b/seatd/client.c
@@ -48,6 +48,9 @@ static int get_peer(int fd, pid_t *pid, uid_t *uid, gid_t *gid) {
	*uid = cred.cr_uid;
	*gid = cred.cr_ngroups > 0 ? cred.cr_groups[0] : (gid_t)-1;
	return 0;
#elif defined(__OpenBSD__)
	*pid = -1;
	return getpeereid(fd, uid, gid);
#else
#error Unsupported platform
#endif
diff --git a/seatd/seat.c b/seatd/seat.c
index 354273fbe41f..8befa3e17947 100644
--- a/seatd/seat.c
+++ b/seatd/seat.c
@@ -11,13 +11,16 @@

#include "client.h"
#include "drm.h"
#include "evdev.h"
#include "linked_list.h"
#include "log.h"
#include "protocol.h"
#include "seat.h"
#include "terminal.h"

#if defined(__linux__) || defined(__FreeBSD__)
#include "evdev.h"
#endif

static int seat_close_client(struct client *client);
static int vt_close(int vt);

@@ -231,9 +234,12 @@ struct seat_device *seat_open_device(struct client *client, const char *path) {
	}

	enum seat_device_type type;
#if defined(__linux__) || defined(__FreeBSD__)
	if (path_is_evdev(sanitized_path)) {
		type = SEAT_DEVICE_TYPE_EVDEV;
	} else if (path_is_drm(sanitized_path)) {
	} else
#endif
	if (path_is_drm(sanitized_path)) {
		type = SEAT_DEVICE_TYPE_DRM;
	} else {
		log_errorf("%s is not a supported device type ", sanitized_path);
@@ -328,10 +334,12 @@ static int seat_deactivate_device(struct seat_device *seat_device) {
		}
		break;
	case SEAT_DEVICE_TYPE_EVDEV:
#if defined(__linux__) || defined(__FreeBSD__)
		if (evdev_revoke(seat_device->fd) == -1) {
			log_errorf("Could not revoke evdev on device fd: %s", strerror(errno));
			return -1;
		}
#endif
		break;
	default:
		log_error("Invalid seat device type");
diff --git a/tests/poller.c b/tests/poller.c
index 382d9d2a7ab3..f44f82cd5b35 100644
--- a/tests/poller.c
+++ b/tests/poller.c
@@ -8,6 +8,10 @@
#include "poller.h"
#include "test.h"

#ifdef __OpenBSD__
#define SIGRTMIN SIGUSR1
#endif

static void test_poller_init(void) {
	struct poller poller;
	test_assert(poller_init(&poller) == 0);
-- 
2.33.1
Neat! I suppose some of the wscons-related changes could also be 
applied to NetBSD. Drew linked swc as a source of inspiration in 
https://todo.sr.ht/~kennylevinsen/seatd/11.

Could you add an OpenBSD build manifest when you get around to it?

On Sat, Oct 23 2021 at 02:27:41 PM +0000, Simon Ser 
<contact@emersion.fr> wrote: