[DTrace-devel] [PATCH 01/14] No longer depend on libsystemd
Kris Van Hees
kris.van.hees at oracle.com
Fri Oct 25 02:21:26 UTC 2024
On Thu, Oct 24, 2024 at 12:37:45PM +0100, Nick Alcock wrote:
> We only need this for the systemd notification protocol, and that's so
> simple there is MIT-0-licensed reusable code in the sd_notify manpage
> showing you how to use it. That code looks pretty much OK, so I modified it
> lightly and pulled it into libport, with one tweak to remove the use of
> __attribute__((cleanup)) (it could be replaced by one line of perfectly
> trivial standard C code, saving five or six lines in all).
>
> With libsystemd linkage gone, we need another way for the user to indicate
> that systemd is not in use and the systemd unit files are not wanted: so
> introduce a new --without-systemd (and corresponding --with-systemd=no,
> etc) in configure. (Someone directly running make can just pass
> WITH_SYSTEMD= to make.)
>
> We lose one feature of libsystemd's sd_notify: we no longer remove the
> NOTIFY_SOCKET env var from the environment, so our children could in theory
> pretend to be us and notify systemd on our behalf. Since our only child
> is in a seccomped jail and is part of dtprobed anyway, this loss of
> functionality is purely theoretical.
>
> Bug: https://github.com/oracle/dtrace-utils/issues/92
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
> GNUmakefile | 1 +
> Makeconfig | 1 -
> configure | 8 +++--
> dtprobed/Build | 7 +---
> dtprobed/dtprobed.c | 8 +----
> include/port.h | 1 +
> libport/Build | 4 +--
> libport/systemd_notify.c | 70 ++++++++++++++++++++++++++++++++++++++++
> 8 files changed, 82 insertions(+), 18 deletions(-)
> create mode 100644 libport/systemd_notify.c
>
> diff --git a/GNUmakefile b/GNUmakefile
> index e7657d466df5..99960227bcba 100644
> --- a/GNUmakefile
> +++ b/GNUmakefile
> @@ -102,6 +102,7 @@ PKGCONFIGDIR = $(prefix)/share/pkgconfig
> INSTPKGCONFIGDIR = $(DESTDIR)$(PKGCONFIGDIR)
> TESTDIR = $(LIBDIR)/dtrace/testsuite
> INSTTESTDIR = $(DESTDIR)$(TESTDIR)
> +WITH_SYSTEMD = true
Why not: WITH_SYSTEMD = y
(since everything else seems to use = y)
> TARGETS =
>
> DTRACE ?= $(objdir)/dtrace
> diff --git a/Makeconfig b/Makeconfig
> index f3d47c086f4a..346078598624 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -218,7 +218,6 @@ $(eval $(call check-symbol-rule,ELF_GETSHDRSTRNDX,elf_getshdrstrndx,elf))
> $(eval $(call check-symbol-rule,LIBCTF,ctf_open,ctf,t))
> $(eval $(call check-symbol-rule,STRRSTR,strrstr,c))
> $(eval $(call check-symbol-rule,PTHREAD_ATFORK,pthread_atfork,c))
> -$(eval $(call check-symbol-rule,LIBSYSTEMD,sd_notify,systemd,t))
> ifndef WANTS_LIBFUSE2
> $(eval $(call check-symbol-rule,FUSE_LOG,fuse_set_log_func,fuse3))
> $(eval $(call check-symbol-rule,LIBFUSE3,fuse_session_receive_buf,fuse3,t))
> diff --git a/configure b/configure
> index 4026a3eafcd3..42e496b97a7d 100755
> --- a/configure
> +++ b/configure
> @@ -93,9 +93,13 @@ built locally, none of these should be needed):
>
> EOF
> make help-overrides-header help-overrides-option
> + cat >&2 <<'EOF'
> +--with-systemd=[yes/no] Disable installation of the systemd unit files
This makes no sense to me. How would '--with-systemd' mean that installation
of the systemd unit files is to be disabled? Surely that is not the correct
description? Maybe something like: "Enable or disable ..." or
"Specify whether ..."?
> +EOF
> echo >&2
> make help-overrides
> cat >&2 <<'EOF'
> +
> Options controlling the compiler (pass on the command line):
>
> CC C compiler (may be a cross-compiler)
> @@ -153,12 +157,12 @@ for option in "$@"; do
> --kernel-obj-dir=*) write_make_var KERNELOBJDIR "$option";;
> --kernel-src-suffix=*) write_make_var KERNELSRCNAME "$option";;
> --kernel-obj-suffix=*) write_make_var KERNELBLDNAME "$option";;
> + --with-systemd|--with-systemd=yes) write_make_var WITH_SYSTEMD "true";;
"true" -> "y"
> + --with-systemd=no|--without-systemd) write_make_var WITH_SYSTEMD "";;
> HAVE_ELF_GETSHDRSTRNDX=*) write_config_var ELF_GETSHDRSTRNDX "$option";;
> --with-libctf=*) write_config_var LIBCTF "$option";;
> HAVE_LIBCTF=*) write_config_var LIBCTF "$option";;
> HAVE_STRRSTR=*) write_config_var STRRSTR "$option";;
> - --with-libsystemd=*) write_config_var LIBSYSTEMD "$option";;
> - HAVE_LIBSYSTEMD=*) write_config_var LIBSYSTEMD "$option";;
> HAVE_FUSE_LOG=*) write_config_var FUSE_LOG "$option";;
> --with-libfuse3=*) write_config_var LIBFUSE3 "$option";;
> HAVE_LIBFUSE3=*) write_config_var LIBFUSE3 "$option";;
> diff --git a/dtprobed/Build b/dtprobed/Build
> index 9132c3e31c8a..346cd6aee190 100644
> --- a/dtprobed/Build
> +++ b/dtprobed/Build
> @@ -29,11 +29,6 @@ dtprobed_DEPS := libproc.a libcommon.a libport.a
> dtprobed_SOURCES := dtprobed.c dof_stash.c seccomp-assistance.c
> dtprobed_LIBSOURCES := libproc libcommon
>
> -ifdef HAVE_LIBSYSTEMD
> -dtprobed_CFLAGS += $(shell pkg-config --cflags libsystemd)
> -dtprobed_LIBS += $(shell pkg-config --libs libsystemd)
> -endif
> -
> ifndef HAVE_FUSE_LOG
> dtprobed_SOURCES += rpl_fuse_log.c
> endif
> @@ -43,7 +38,7 @@ seccomp-assistance.c_CFLAGS := -fno-lto
> endif
>
> install-dtprobed-autostart::
> -ifdef HAVE_LIBSYSTEMD
> +ifneq ($(WITH_SYSTEMD),)
> mkdir -p $(INSTSYSTEMDUNITDIR) $(INSTSYSTEMDPRESETDIR)
> $(call describe-install-target,$(INSTSYSTEMDUNITDIR),dtprobed.service)
> sed 's, at SBINDIR@,$(SBINDIR),' < $(dtprobed_DIR)dtprobed.service.in > $(INSTSYSTEMDUNITDIR)/dtprobed.service
> diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
> index fdcdee14f851..c83f0adb56c4 100644
> --- a/dtprobed/dtprobed.c
> +++ b/dtprobed/dtprobed.c
> @@ -61,10 +61,6 @@
>
> #include <dtrace/ioctl.h>
>
> -#ifdef HAVE_LIBSYSTEMD
> -#include <systemd/sd-daemon.h>
> -#endif
> -
> #include <dt_list.h>
> #include "dof_parser.h"
> #include "dof_stash.h"
> @@ -1073,9 +1069,7 @@ main(int argc, char *argv[])
> sync_fd = -1;
> }
>
> -#ifdef HAVE_LIBSYSTEMD
> - sd_notify(1, "READY=1");
> -#endif
> + systemd_notify("READY=1");
>
> ret = loop();
>
> diff --git a/include/port.h b/include/port.h
> index 427129000b4f..6ce8611e9025 100644
> --- a/include/port.h
> +++ b/include/port.h
> @@ -29,6 +29,7 @@ int p_online(int cpun);
> #define MUTEX_HELD(x) ((x)->__data.__count == 0)
>
> int daemonize(int close_fds);
> +int systemd_notify(const char *message);
>
> _dt_noreturn_ void daemon_perr(int fd, const char *err, int err_no);
> _dt_printflike_(2, 3) void daemon_log(int fd, const char *fmt, ...);
> diff --git a/libport/Build b/libport/Build
> index 48ed99e2b017..822049f5e547 100644
> --- a/libport/Build
> +++ b/libport/Build
> @@ -1,5 +1,5 @@
> # Oracle Linux DTrace.
> -# Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
> +# Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved.
> # Licensed under the Universal Permissive License v 1.0 as shown at
> # http://oss.oracle.com/licenses/upl.
>
> @@ -8,7 +8,7 @@ LIBS += libport
>
> libport_TARGET = libport
> libport_DIR := $(current-dir)
> -libport_SOURCES = gmatch.c linux_version_code.c strlcat.c strlcpy.c p_online.c time.c daemonize.c
> +libport_SOURCES = gmatch.c linux_version_code.c strlcat.c strlcpy.c p_online.c time.c daemonize.c systemd_notify.c
> ifndef HAVE_CLOSE_RANGE
> libport_SOURCES += close_range.c
> endif
> diff --git a/libport/systemd_notify.c b/libport/systemd_notify.c
> new file mode 100644
> index 000000000000..f78bca6e14c5
> --- /dev/null
> +++ b/libport/systemd_notify.c
> @@ -0,0 +1,70 @@
> +/* SPDX-License-Identifier: MIT-0 */
> +/* Lightly modified from the sd_notify manpage. */
> +
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <stddef.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +int
> +systemd_notify(const char *message)
> +{
> + union sockaddr_union {
> + struct sockaddr sa;
> + struct sockaddr_un sun;
> + } socket_addr = {
> + .sun.sun_family = AF_UNIX,
> + };
> +
> + size_t path_length, message_length;
> + int fd = -1;
> + const char *socket_path;
> +
> + /* Verify the argument first */
> + if (!message)
> + return -EINVAL;
> +
> + message_length = strlen(message);
> + if (message_length == 0)
> + return -EINVAL;
> +
> + /* If the variable is not set, the protocol is a noop */
> + socket_path = getenv("NOTIFY_SOCKET");
> + if (!socket_path)
> + return 0; /* Not set? Nothing to do */
> +
> + /* Only AF_UNIX is supported, with path or abstract sockets */
> + if (socket_path[0] != '/' && socket_path[0] != '@')
> + return -EAFNOSUPPORT;
> +
> + path_length = strlen(socket_path);
> + /* Ensure there is room for NUL byte */
> + if (path_length >= sizeof(socket_addr.sun.sun_path))
> + return -E2BIG;
> +
> + memcpy(socket_addr.sun.sun_path, socket_path, path_length);
> +
> + /* Support for abstract socket */
> + if (socket_addr.sun.sun_path[0] == '@')
> + socket_addr.sun.sun_path[0] = 0;
> +
> + fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0);
> + if (fd < 0)
> + return -errno;
> +
> + if (connect(fd, &socket_addr.sa, offsetof(struct sockaddr_un, sun_path) + path_length) != 0) {
> + close(fd);
> + return -errno;
> + }
> +
> + ssize_t written = write(fd, message, message_length);
> + if (written != (ssize_t) message_length) {
> + close(fd);
> + return written < 0 ? -errno : -EPROTO;
> + }
> +
> + return 1; /* Notified! */
> +}
> --
> 2.46.0.278.g36e3a12567
>
More information about the DTrace-devel
mailing list