Hi Andrew, On Tue, Apr 29, 2025 at 10:15 AM Andrew Sayers <kernel.org@xxxxxxxxxxxxxxx> wrote: > > Obexd is usually run as a user service, and can exhibit surprising > behaviour if two users are logged in at the same time. > > Unregister profiles when the user is detected to be off-seat. > > It may be impossible to detect whether a user is on-seat in some cases. > For example, a version of obexd compiled with systemd support might be > run outside of a systemd environment. Warn and leave services > registered if that happens. > > Obexd can be run as a system service, in which case this check makes no > sense. Disable this check when called with `--system-bus`. > > Obexd can also be run by a user that does not have an active session. > For example, someone could use `ssh` to access the system. There might > be a use case where someone needs Bluetooth access but can't log in with > a keyboard, or there might be a security issue with doing so. This isn't > handled explicitly by this patch, but a future patch could add support > by calling `logind_set(FALSE)` in the same way as is currently done > with `--system-bus`. > > Unregister profiles by closing private connections instead of sending > UnregisterProfile on the shared connection. Pipewire has apparently > found the latter to cause long shutdown delays, because bluetoothd > may be shutting down and unable to handle this message. > > Based in large part on the wireplumber code mentioned by Pauli Virtanen: > https://gitlab.freedesktop.org/pipewire/wireplumber/-/blob/master/modules/module-logind.c#L52 > > Other services are likely to need similar functionality, > so I have created a gist to demonstrate the basic technique: > https://gist.github.com/andrew-sayers/1c4a24f86a9a4c1b1e38d109f1bd1d1e > > Suggested-by: Pauli Virtanen <pav@xxxxxx> > Signed-off-by: Andrew Sayers <kernel.org@xxxxxxxxxxxxxxx> > --- > Makefile.obexd | 10 ++ > obexd/client/pbap.c | 17 ++- > obexd/plugins/bluetooth.c | 14 ++- > obexd/src/logind.c | 239 ++++++++++++++++++++++++++++++++++++++ > obexd/src/logind.h | 26 +++++ > obexd/src/main.c | 4 + > 6 files changed, 305 insertions(+), 5 deletions(-) > create mode 100644 obexd/src/logind.c > create mode 100644 obexd/src/logind.h > > diff --git a/Makefile.obexd b/Makefile.obexd > index 74dd977a0..b59cfaf8f 100644 > --- a/Makefile.obexd > +++ b/Makefile.obexd > @@ -67,6 +67,7 @@ obexd_src_obexd_SOURCES = $(btio_sources) $(gobex_sources) \ > obexd/src/main.c obexd/src/obexd.h \ > obexd/src/plugin.h obexd/src/plugin.c \ > obexd/src/log.h obexd/src/log.c \ > + obexd/src/logind.h obexd/src/logind.c \ > obexd/src/manager.h obexd/src/manager.c \ > obexd/src/obex.h obexd/src/obex.c obexd/src/obex-priv.h \ > obexd/src/mimetype.h obexd/src/mimetype.c \ > @@ -96,6 +97,8 @@ obexd_src_obexd_LDADD = lib/libbluetooth-internal.la \ > > if EXTERNAL_PLUGINS > obexd_src_obexd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic > +else > +obexd_src_obexd_LDFLAGS = > endif > > obexd_src_obexd_CPPFLAGS = $(AM_CPPFLAGS) $(GLIB_CFLAGS) $(DBUS_CFLAGS) \ > @@ -109,6 +112,13 @@ obexd-add-service-symlink: > obexd-remove-service-symlink: > endif > > +if OBEX > +if SYSTEMD > +obexd_src_obexd_CPPFLAGS += -DSYSTEMD > +obexd_src_obexd_LDFLAGS += -lsystemd > +endif > +endif > + > obexd_src_obexd_SHORTNAME = obexd > > obexd_builtin_files = obexd/src/builtin.h $(obexd_builtin_nodist) > diff --git a/obexd/client/pbap.c b/obexd/client/pbap.c > index 90f8bdc02..51b523592 100644 > --- a/obexd/client/pbap.c > +++ b/obexd/client/pbap.c > @@ -27,6 +27,7 @@ > #include "gdbus/gdbus.h" > > #include "obexd/src/log.h" > +#include "obexd/src/logind.h" > #include "obexd/src/obexd.h" > > #include "transfer.h" > @@ -1454,13 +1455,13 @@ static struct obc_driver pbap = { > .remove = pbap_remove > }; > > -int pbap_init(void) > +static int pbap_init_cb(void) > { > int err; > > DBG(""); > > - conn = obex_get_dbus_connection(); > + conn = obex_setup_dbus_connection_private(NULL, NULL); > if (!conn) > return -EIO; > > @@ -1481,7 +1482,7 @@ int pbap_init(void) > return 0; > } > > -void pbap_exit(void) > +static void pbap_exit_cb(void) > { > DBG(""); > > @@ -1496,9 +1497,19 @@ void pbap_exit(void) > } > > if (conn) { > + dbus_connection_close(conn); > dbus_connection_unref(conn); > conn = NULL; > } > > obc_driver_unregister(&pbap); > } > + > +int pbap_init(void) > +{ > + return logind_register(pbap_init_cb, pbap_exit_cb); > +} > +void pbap_exit(void) > +{ > + return logind_unregister(pbap_init_cb, pbap_exit_cb); > +} > diff --git a/obexd/plugins/bluetooth.c b/obexd/plugins/bluetooth.c > index 8cf718922..7ff27a8a8 100644 > --- a/obexd/plugins/bluetooth.c > +++ b/obexd/plugins/bluetooth.c > @@ -35,6 +35,7 @@ > #include "obexd/src/transport.h" > #include "obexd/src/service.h" > #include "obexd/src/log.h" > +#include "obexd/src/logind.h" > > #define BT_RX_MTU 32767 > #define BT_TX_MTU 32767 > @@ -426,7 +427,7 @@ static const struct obex_transport_driver driver = { > > static unsigned int listener_id = 0; > > -static int bluetooth_init(void) > +static int bluetooth_init_cb(void) > { > connection = g_dbus_setup_private(DBUS_BUS_SYSTEM, NULL, NULL); > if (connection == NULL) > @@ -438,7 +439,7 @@ static int bluetooth_init(void) > return obex_transport_driver_register(&driver); > } > > -static void bluetooth_exit(void) > +static void bluetooth_exit_cb(void) > { > GSList *l; > > @@ -462,4 +463,13 @@ static void bluetooth_exit(void) > obex_transport_driver_unregister(&driver); > } > > +static int bluetooth_init(void) > +{ > + return logind_register(bluetooth_init_cb, bluetooth_exit_cb); > +} > +static void bluetooth_exit(void) > +{ > + return logind_unregister(bluetooth_init_cb, bluetooth_exit_cb); > +} > + > OBEX_PLUGIN_DEFINE(bluetooth, bluetooth_init, bluetooth_exit) > diff --git a/obexd/src/logind.c b/obexd/src/logind.c > new file mode 100644 > index 000000000..ff2bf3219 > --- /dev/null > +++ b/obexd/src/logind.c > @@ -0,0 +1,239 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * > + * Enable functionality only when the user is active > + * > + * Copyright (C) 2007-2010 Marcel Holtmann <marcel@xxxxxxxxxxxx> > + * > + * > + */ > + > +#ifdef SYSTEMD > + > +#include <assert.h> > +#include <errno.h> > +#include <poll.h> > +#include <stddef.h> > +#include <stdlib.h> > +#include <string.h> > +#include <time.h> > +#include <unistd.h> > +#include <glib.h> > + > +#include <systemd/sd-login.h> > + > +#include "obexd/src/log.h" > +#include "obexd/src/logind.h" > + > +static sd_login_monitor * monitor; > +static int uid; > +static gboolean active = FALSE; > +static gboolean monitoring_enabled = TRUE; > +static guint event_source; > + > +struct callback_pair { > + logind_init_cb init_cb; > + logind_exit_cb exit_cb; > +}; > + > +GSList *callbacks; > + > +static void call_init_cb(gpointer data, gpointer user_data) > +{ > + int res; > + > + res = ((struct callback_pair *)data)->init_cb(); > + if (res) > + *(int *)user_data = res; > +} > +static void call_exit_cb(gpointer data, gpointer user_data) > +{ > + ((struct callback_pair *)data)->exit_cb(); > +} > + > +static int update(void) > +{ > + char *state = NULL; > + gboolean state_is_active; > + int res; > + > + res = sd_login_monitor_flush(monitor); > + if (res < 0) > + return res; > + res = sd_uid_get_state(uid, &state); > + state_is_active = g_strcmp0(state, "active"); > + free(state); > + if (res < 0) > + return res; > + > + if (state_is_active) { > + if (!active) > + return 0; > + } else { > + res = sd_uid_get_seats(uid, 1, NULL); > + if (res < 0) > + return res; > + if (active == !!res) > + return 0; > + } > + active ^= TRUE; > + res = 0; > + g_slist_foreach(callbacks, active ? call_init_cb : call_exit_cb, &res); > + return res; > +} > + > +static int check_event(void) > +{ > + int res; > + > + res = sd_login_monitor_flush(monitor); > + if (res < 0) > + return res; > + if (!monitoring_enabled) > + return 0; > + res = update(); > + if (res < 0) > + return res; > + > + return 0; > +} > + > + > +static gboolean event_handler(GIOChannel *source, GIOCondition condition, > + gpointer data) > +{ > + int res; > + > + res = check_event(); > + if (res) { > + error("%s: %s", __func__, strerror(-res)); > + return FALSE; > + } > + > + return TRUE; > +} > + > +static int logind_init(void) > +{ > + GIOChannel *channel; > + int events; > + int fd; > + int res; > + > + monitor = NULL; > + > + DBG(""); > + > + if (!monitoring_enabled) > + return 0; > + > + uid = getuid(); > + > + res = sd_login_monitor_new("uid", &monitor); > + if (res < 0) { > + monitor = NULL; > + goto FAIL; > + } > + > + // Check this after creating the monitor, in case of race conditions: > + res = update(); > + if (res < 0) > + goto FAIL; > + > + events = res = sd_login_monitor_get_events(monitor); > + if (res < 0) > + goto FAIL; > + > + fd = res = sd_login_monitor_get_fd(monitor); > + if (res < 0) > + goto FAIL; > + > + channel = g_io_channel_unix_new(fd); > + > + g_io_channel_set_close_on_unref(channel, TRUE); > + g_io_channel_set_encoding(channel, NULL, NULL); > + g_io_channel_set_buffered(channel, FALSE); > + > + event_source = g_io_add_watch(channel, events, event_handler, NULL); > + > + g_io_channel_unref(channel); > + > + return check_event(); > + > +FAIL: > + sd_login_monitor_unref(monitor); > + monitoring_enabled = FALSE; > + active = TRUE; > + return res; > +} > + > +static void logind_exit(void) > +{ > + if (event_source) { > + g_source_remove(event_source); > + event_source = 0; > + } > + sd_login_monitor_unref(monitor); > +} > + > +static gint find_cb(gconstpointer a, gconstpointer b) > +{ > + return ((struct callback_pair *)a)->init_cb - (logind_init_cb)b; > +} > + > +int logind_register(logind_init_cb init_cb, logind_exit_cb exit_cb) > +{ > + struct callback_pair *cbs; > + > + if (!monitoring_enabled) > + return init_cb(); > + if (callbacks == NULL) { > + int res; > + > + res = logind_init(); > + if (res) { > + error("logind_init(): %s - login detection disabled", > + strerror(-res)); > + return init_cb(); > + } > + } > + cbs = g_new(struct callback_pair, 1); > + cbs->init_cb = init_cb; > + cbs->exit_cb = exit_cb; > + callbacks = g_slist_prepend(callbacks, cbs); > + return active ? init_cb() : 0; > +} > +void logind_unregister(logind_init_cb init_cb, logind_exit_cb exit_cb) > +{ > + GSList *cb_node; > + > + if (!monitoring_enabled) > + return exit_cb(); > + if (active) > + exit_cb(); > + cb_node = g_slist_find_custom(callbacks, init_cb, find_cb); > + if (cb_node != NULL) > + callbacks = g_slist_delete_link(callbacks, cb_node); > + if (callbacks == NULL) > + logind_exit(); > +} > + > +int logind_set(gboolean enabled) > +{ > + int res = 0; > + > + if (monitoring_enabled == enabled) > + return 0; > + > + monitoring_enabled = enabled; > + if (enabled) { > + active = FALSE; > + return update(); > + } > + > + active = TRUE; > + g_slist_foreach(callbacks, call_exit_cb, &res); > + return res; > +} > + > +#endif > diff --git a/obexd/src/logind.h b/obexd/src/logind.h > new file mode 100644 > index 000000000..1a92a8b87 > --- /dev/null > +++ b/obexd/src/logind.h > @@ -0,0 +1,26 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * > + * Enable functionality only when the user is active > + * > + * Copyright (C) 2007-2010 Marcel Holtmann <marcel@xxxxxxxxxxxx> > + * > + * > + */ > + > +#ifdef SYSTEMD > + > +typedef int (*logind_init_cb)(void); > +typedef void (*logind_exit_cb)(void); > + > +int logind_register(logind_init_cb init_cb, logind_exit_cb exit_cb); > +void logind_unregister(logind_init_cb init_cb, logind_exit_cb exit_cb); > +int logind_set(gboolean enabled); > + > +#else > + > +#define logind_register(init_cb, exit_cb) init_cb() > +#define logind_unregister(init_cb, exit_cb) exit_cb() > +#define logind_set(enabled) 0 > + > +#endif > diff --git a/obexd/src/main.c b/obexd/src/main.c > index ca95a70de..df150973e 100644 > --- a/obexd/src/main.c > +++ b/obexd/src/main.c > @@ -35,6 +35,7 @@ > #include "../client/manager.h" > > #include "log.h" > +#include "logind.h" > #include "obexd.h" > #include "server.h" > > @@ -283,6 +284,9 @@ int main(int argc, char *argv[]) > > __obex_log_init(option_debug, option_detach); > > + if (option_system_bus) > + logind_set(FALSE); > + > DBG("Entering main loop"); > > main_loop = g_main_loop_new(NULL, FALSE); > -- > 2.49.0 Applying: obexd: Unregister profiles when the user is inactive WARNING:MACRO_ARG_UNUSED: Argument 'exit_cb' is not used in function-like macro #409: FILE: obexd/src/logind.h:22: +#define logind_register(init_cb, exit_cb) init_cb() WARNING:MACRO_ARG_UNUSED: Argument 'init_cb' is not used in function-like macro #410: FILE: obexd/src/logind.h:23: +#define logind_unregister(init_cb, exit_cb) exit_cb() WARNING:MACRO_ARG_UNUSED: Argument 'enabled' is not used in function-like macro #411: FILE: obexd/src/logind.h:24: +#define logind_set(enabled) 0 -- Luiz Augusto von Dentz
