From 892a24d42caeaf79eda8b5e8ae96368f44f2448e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Jul 2025 07:04:09 +0200 Subject: [PATCH 1/9] shared/bus-unit-util: check errors before other conditions As requested in post-merge review in https://github.com/systemd/systemd/pull/37665#discussion_r2183755909. --- src/shared/bus-unit-util.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index c5e02a912f..9a4a8a865a 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -375,10 +375,10 @@ static int bus_append_parse_cpu_quota(sd_bus_message *m, const char *field, cons x = USEC_INFINITY; else { r = parse_permyriad_unbounded(eq); - if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(ERANGE), "%s value too small.", field); if (r < 0) return log_error_errno(r, "Failed to parse %s=%s: %m", field, eq); + if (r == 0) + return log_error_errno(SYNTHETIC_ERRNO(ERANGE), "%s value too small.", field); x = r * USEC_PER_SEC / 10000U; } @@ -864,12 +864,12 @@ static int bus_append_parse_ip_address_filter(sd_bus_message *m, const char *fie _cleanup_free_ char *word = NULL; r = extract_first_word(&eq, &word, NULL, 0); - if (r == 0) - break; if (r == -ENOMEM) return log_oom(); if (r < 0) return log_error_errno(r, "Failed to parse %s: %s", field, eq); + if (r == 0) + break; r = in_addr_prefix_from_string_auto(word, &family, &prefix, &prefixlen); if (r < 0) @@ -1434,12 +1434,12 @@ static int bus_append_filter_list(sd_bus_message *m, const char *field, const ch _cleanup_free_ char *word = NULL; r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE); - if (r == 0) - break; if (r == -ENOMEM) return log_oom(); if (r < 0) return log_error_errno(r, "Invalid syntax: %s", eq); + if (r == 0) + break; r = sd_bus_message_append_basic(m, 's', word); if (r < 0) @@ -2127,12 +2127,12 @@ static int bus_append_exit_status(sd_bus_message *m, const char *field, const ch _cleanup_free_ char *word = NULL; r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE); - if (r == 0) - break; if (r == -ENOMEM) return log_oom(); if (r < 0) return log_error_errno(r, "Invalid syntax in %s: %s", field, eq); + if (r == 0) + break; /* We need to call exit_status_from_string() first, because we want * to parse numbers as exit statuses, not signals. */ From 743b09c42ccfed2f44a155654f042b3eef7ce71d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 3 Jul 2025 18:52:26 +0200 Subject: [PATCH 2/9] test-bus-unit-util: add a test that attempts to serialize all know transient settings The samples were partially generated using claude.ai. Those examples are usually fairly boring. I tried to remove obvious repetitions and add some more interesting examples, but certainly more edge cases could be added. In some cases, we are quite lenient and do almost no verification on the sender side. --- src/test/test-bus-unit-util.c | 1156 ++++++++++++++++++++++++++++++++- 1 file changed, 1154 insertions(+), 2 deletions(-) diff --git a/src/test/test-bus-unit-util.c b/src/test/test-bus-unit-util.c index a1f0104bc0..a5207fa02d 100644 --- a/src/test/test-bus-unit-util.c +++ b/src/test/test-bus-unit-util.c @@ -1,15 +1,1167 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "sd-bus.h" + #include "bus-unit-util.h" +#include "extract-word.h" #include "unit-def.h" +#include "strv.h" #include "tests.h" +static sd_bus *arg_bus = NULL; +STATIC_DESTRUCTOR_REGISTER(arg_bus, sd_bus_unrefp); + +static void test_transient_settings_one(UnitType type, const char* const* lines) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; + int r; + + if (!arg_bus) + return (void) log_tests_skipped("no bus connection"); + + ASSERT_OK(sd_bus_message_new(arg_bus, &m, SD_BUS_MESSAGE_METHOD_CALL)); + + STRV_FOREACH(s, lines) { + const char *t = *s; + int expect = 1; + + if (t[0] == '-') { + _cleanup_free_ char *code = NULL; + ASSERT_OK(extract_first_word(&t, &code, " ", 0)); + ASSERT_OK(r = errno_from_name(code + 1)); + expect = -r; + } + + r = bus_append_unit_property_assignment(m, type, t); + log_debug("%s → %d/%s", t, r, r < 0 ? ERRNO_NAME_FULL(r) : yes_no(r)); + ASSERT_EQ(r, expect); + } +} + +/* The tests data below is in a format intended to be easy to read and write: + * Examples can be plain or prefixed with a negative numeric error code: + * - unadorned examples must return 1 (success), + * - otherwise, the function must return the given error. + * Note that some functions leave the message in a broken state and subsequent + * attempts to extend the message will return -ENXIO. + */ + +TEST(cgroup_properties) { + const char* const *lines = STRV_MAKE_CONST( + "DevicePolicy=strict", + "DevicePolicy=auto", + "DevicePolicy=closed", + + "Slice=system.slice", + "Slice=user-1000.slice", + "Slice=system-getty-this-is-a-very-long-long-slice.slice", + "Slice=", + + "ManagedOOMSwap=auto", + "ManagedOOMSwap=kill", + "ManagedOOMSwap=", + + "ManagedOOMMemoryPressure=auto", + "ManagedOOMMemoryPressure=kill", + "ManagedOOMMemoryPressure=", + + "ManagedOOMPreference=none", + "ManagedOOMPreference=avoid", + "ManagedOOMPreference=omit", + + "MemoryPressureWatch=auto", + "MemoryPressureWatch=off", + "MemoryPressureWatch=on", + "MemoryPressureWatch=skip", + + "DelegateSubgroup=foo", + "DelegateSubgroup=bar.scope", + "DelegateSubgroup=", + + "MemoryAccounting=true", + "-EINVAL MemoryAccounting=false ", + "-EINVAL MemoryAccounting= yes", + "-EINVAL MemoryAccounting=\t\tno\t\t", + "MemoryAccounting=1", + "MemoryAccounting=0", + "MemoryAccounting=on", + "MemoryAccounting=off", + "MemoryAccounting=True", + "MemoryAccounting=FALSE", + "MemoryAccounting=YES", + "MemoryAccounting=No", + + "MemoryZSwapWriteback=true", + "MemoryZSwapWriteback=false", + "IOAccounting=yes", + "IOAccounting=no", + "TasksAccounting=1", + "TasksAccounting=0", + "IPAccounting=on", + "IPAccounting=off", + "CoredumpReceive=true", + "CoredumpReceive=false", + + "CPUWeight=100", + "StartupCPUWeight=100", + "IOWeight=100", + "StartupIOWeight=100", + + "AllowedCPUs=0-3", + "AllowedCPUs=1,3,5,7", + "StartupAllowedCPUs=0-1", + "StartupAllowedCPUs=2,4,6", + + "AllowedMemoryNodes=0", + "AllowedMemoryNodes=0-2", + "AllowedMemoryNodes=0-2 3 5 7 3 2 1", + "StartupAllowedMemoryNodes=0", + "StartupAllowedMemoryNodes=1-3", + + "DisableControllers=cpu", + "DisableControllers= " + " cpu cpuacct cpuset io blkio memory devices pids bpf-firewall bpf-devices " + " cpu cpuacct cpuset io blkio memory\tdevices\tpids\tbpf-firewall\tbpf-devices ", + + "Delegate=yes", + + "MemoryMin=100M", + "DefaultMemoryLow=50M", + "DefaultMemoryMin=10M", + "MemoryLow=2T", + "MemoryHigh=4G", + "MemoryMax=2G", + "MemoryMax=infinity", + "MemorySwapMax=1G", + "MemorySwapMax=0", + "MemoryZSwapMax=500M", + "MemoryZSwapMax=1G", + "TasksMax=1000", + "TasksMax=infinity", + + "CPUQuota=1%", + "CPUQuota=200%", + "CPUQuotaPeriodSec=100ms", + "CPUQuotaPeriodSec=1s", + + "DeviceAllow=/dev/null rw", + "DeviceAllow=/dev/zero r", + + "IODeviceWeight=/dev/sda 200", + "IODeviceWeight=/dev/nvme0n1 500", + "IODeviceLatencyTargetSec=/dev/sda 100ms", + "IODeviceLatencyTargetSec=/dev/nvme0n1 10ms", + + "IPAddressAllow=10.0.0.0/8", + "IPAddressAllow=192.168.1.0/24 ::1", + "IPAddressDeny=0.0.0.0/0", + "IPAddressDeny=192.168.100.0/24", + + "IPIngressFilterPath=/etc/systemd/bpf/ingress.bpf", + "IPEgressFilterPath=/etc/systemd/bpf/egress.bpf", + + "BPFProgram=/usr/lib/systemd/bpf/custom.bpf", + + "SocketBindAllow=tcp:80", + "SocketBindAllow=udp:53", + "SocketBindDeny=tcp:1-1023", + "SocketBindDeny=udp:9999-9999", + "SocketBindDeny=any", + + "MemoryPressureThresholdSec=1s", + "MemoryPressureThresholdSec=1 min 5s 23ms", + + "NFTSet=cgroup:inet:filter:my_service user:inet:filter:serviceuser", + + "ManagedOOMMemoryPressureDurationSec=30s", + "ManagedOOMMemoryPressureDurationSec=infinity", + "ManagedOOMMemoryPressureLimit=0%", + "ManagedOOMMemoryPressureLimit=95.8%", + "ManagedOOMMemoryPressureLimit=95.88%", + "-EINVAL ManagedOOMMemoryPressureLimit=95.888%", /* too many digits after dot */ + "ManagedOOMMemoryPressureLimit=100%", + + "IOReadBandwidthMax=/dev/sda 1M", + "IOReadBandwidthMax=/dev/sdb 2000K", + "IOReadBandwidthMax=/dev/nvme0n1 100M", + "IOReadBandwidthMax=/dev/nvme0n1p1 1G", + "IOReadBandwidthMax=/dev/mapper/root infinity", + "IOWriteBandwidthMax=/dev/sda 1M", + "IOWriteBandwidthMax=/dev/sdb 2M", + "IOWriteBandwidthMax=/dev/nvme0n1 100M", + "IOWriteBandwidthMax=/dev/mapper/home infinity", + + /* Various strange cases */ + "DevicePolicy=", + "DevicePolicy=value=with=equals", + "DevicePolicy=auto domain", + "DevicePolicy=policy with spaces", + "Slice=complex-name_with.various-chars123", + "ManagedOOMSwap=kill immediate", + "ManagedOOMMemoryPressureLimit=50.5%", + "IOReadBandwidthMax=/dev/disk/by-uuid/12345678-1234-1234-1234-123456789012 100M", + "IOWriteBandwidthMax=/dev/disk/by-label/DATA 200M", + "IOReadBandwidthMax=/dev/disk/by-partlabel/EFI\\x20System\\x20Partition 500M", + "IOWriteBandwidthMax=/dev/disk/by-id/dm-uuid-CRYPT-LUKS2-00f7541884c34bff92f60faec9d8f217-luks-00f75418-84c3-4bff-92f6-0faec9d8f217 500M", + "DelegateSubgroup=very.long.subgroup.name.with.dots", + "MemoryPressureWatch=custom-value", + + /* Properties with special characters and edge cases */ + "Slice=user-$(id -u).slice", + "DelegateSubgroup=test@service.scope", + "DevicePolicy=auto,strict", + "ManagedOOMSwap=auto;kill", + + /* Deprecated */ + "MemoryLimit=1G", + "MemoryLimit=infinity", + "MemoryLimit=", + "CPUShares=1024", + "StartupCPUShares=1024", + "BlockIOAccounting=true", + "BlockIOWeight=100", + "StartupBlockIOWeight=1000", + "BlockIODeviceWeight=/dev/sda 500", + "BlockIOReadBandwidth=/dev/sda 1M", + "BlockIOWriteBandwidth=/dev/sda 1M", + "CPUAccounting=true" + ); + + test_transient_settings_one(UNIT_SERVICE, lines); + /* Also UNIT_SOCKET, UNIT_SLICE, UNIT_SCOPE, UNIT_MOUNT. */ +} + +TEST(automount_properties) { + const char* const *lines = STRV_MAKE_CONST( + "Where=/mnt/data", + "Where=/home/user/./../storage", + "Where=non/absolute/path", // TODO: should this be allowed? + + "ExtraOptions=uid=1000,gid=1000", + "ExtraOptions= rw,noatime,user_xattr ", + + "DirectoryMode=0755", + "DirectoryMode=700", + + "TimeoutIdleSec=60s", + "TimeoutIdleSec=1week 1 day 2h 3 minutes 300s 50 ms" + ); + + test_transient_settings_one(UNIT_AUTOMOUNT, lines); +} + +TEST(execute_properties) { + const char* const *lines = STRV_MAKE_CONST( + /* String properties */ + "User=myuser", + "Group=mygroup", + "UtmpIdentifier=myservice", + "UtmpMode=init", + "UtmpMode=login", + "PAMName=login", + "PAMName=system-auth", + "TTYPath=/dev/tty1", + "TTYPath=/dev/pts/0", + "WorkingDirectory=/var/lib/myapp", + "RootDirectory=/srv/container", + "SyslogIdentifier=myservice", + "ProtectSystem=strict", + "ProtectSystem=true", + "ProtectSystem=false", + "ProtectHome=true", + "ProtectHome=read-only", + "ProtectHome=tmpfs", + + "PrivateTmpEx=true", + "PrivateTmpEx=false", + "PrivateUsersEx=true", + "PrivateUsersEx=false", + "ProtectControlGroupsEx=true", + "ProtectControlGroupsEx=false", + + "SELinuxContext=system_u:system_r:httpd_t:s0", + "RootImage=/var/lib/machines/container.raw", + "RootVerity=/var/lib/machines/container.verity", + "RuntimeDirectoryPreserve=true", + "RuntimeDirectoryPreserve=restart", + "Personality=x86-64", + "Personality=x86", + "KeyringMode=inherit", + "KeyringMode=private", + "KeyringMode=shared", + "ProtectProc=invisible", + "ProtectProc=noaccess", + "ProtectProc=ptraceable", + "ProcSubset=pid", + "ProcSubset=all", + "NetworkNamespacePath=/proc/123/ns/net", + "IPCNamespacePath=/proc/123/ns/ipc", + "LogNamespace=myapp", + "RootImagePolicy=closed", + "RootImagePolicy=strict", + "MountImagePolicy=closed", + "MountImagePolicy=strict", + "ExtensionImagePolicy=closed", + "ExtensionImagePolicy=strict", + + /* PrivatePIDs is boolean, but a string. */ + "PrivatePIDs=true", + "PrivatePIDs=false", + "PrivatePIDs=foooooooooo", + + /* Boolean properties */ + "IgnoreSIGPIPE=true", + "TTYVHangup=true", + "TTYReset=true", + "TTYVTDisallocate=true", + "PrivateTmp=true", + "PrivateDevices=true", + "PrivateNetwork=true", + "PrivateUsers=true", + "PrivateMounts=true", + "PrivateIPC=true", + "NoNewPrivileges=true", + "SyslogLevelPrefix=true", + "MemoryDenyWriteExecute=true", + "RestrictRealtime=true", + "DynamicUser=true", + "RemoveIPC=true", + "ProtectKernelTunables=true", + "ProtectKernelModules=true", + "ProtectKernelLogs=true", + "ProtectClock=true", + "ProtectControlGroups=true", + "MountAPIVFS=true", + "BindLogSockets=true", + "CPUSchedulingResetOnFork=true", + "LockPersonality=true", + "MemoryKSM=true", + "RestrictSUIDSGID=true", + "RootEphemeral=true", + "SetLoginEnvironment=true", + + /* Path and directory strvs */ + "ReadWriteDirectories=/var/lib/myapp", + "ReadWriteDirectories=/var/lib/myapp /tmp/workdir", + "ReadOnlyDirectories=/usr/share/data", + "ReadOnlyDirectories=/usr/share/data /etc/config", + "InaccessibleDirectories=/home /root", + "ReadWritePaths=/var/lib/myapp /tmp/workdir", + "ReadOnlyPaths=", + "ReadOnlyPaths=/ /./ /../../ /usr/share/data /etc/config", + "InaccessiblePaths=/home", + "InaccessiblePaths=/home /root /var/cache", + "ExecPaths=/usr/bin /usr/local/bin", + "NoExecPaths=/tmp /var/tmp", + "ExecSearchPath=/usr/bin:/usr/local/bin", + "ExtensionDirectories=/var/lib/extensions /opt/extensions", + "ConfigurationDirectory=myapp subdir", + "SupplementaryGroups=wheel audio video", + "SystemCallArchitectures=native x86-64", + + /* Log levels and facilities */ + "SyslogLevel=info", + "SyslogLevel=debug", + "SyslogLevel=warning", + "SyslogLevel=err", + "-EINVAL SyslogLevel=error", + "LogLevelMax=info", + "LogLevelMax=debug", + "LogLevelMax=warning", + "SyslogFacility=daemon", + "SyslogFacility=local0", + "SyslogFacility=mail", + + /* Various */ + "SecureBits=keep-caps", + "SecureBits=keep-caps-locked", + "SecureBits=no-setuid-fixup", + + "CPUSchedulingPolicy=other", + "CPUSchedulingPolicy=fifo", + "CPUSchedulingPolicy=rr", + "CPUSchedulingPriority=50", + "CPUSchedulingPriority=1", + "CPUSchedulingPriority=99", + + "OOMScoreAdjust=100", + "OOMScoreAdjust=-100", + "OOMScoreAdjust=0", + "CoredumpFilter=0x33", + "CoredumpFilter=0x7f", + "Nice=10", + "Nice=-10", + "SystemCallErrorNumber=EPERM", + "SystemCallErrorNumber=EACCES", + "SystemCallErrorNumber=kill", + + "IOSchedulingClass=1", + "IOSchedulingPriority=4", + + "RuntimeDirectoryMode=0755", + "RuntimeDirectoryMode=750", + "StateDirectoryMode=0000", + "CacheDirectoryMode=0755", + "LogsDirectoryMode=0755", + "ConfigurationDirectoryMode=0755", + "UMask=0002", + + "TimerSlackNSec=50ms", + "LogRateLimitIntervalSec=30s", + "LogRateLimitBurst=1000", + + "TTYRows=24", + "TTYColumns=80", + + "MountFlags=shared", + "MountFlags=slave", + "MountFlags=private", + + "Environment=PATH=/usr/bin:/bin", + "Environment=HOME=/home/user USER=myuser", + "UnsetEnvironment=TEMP", + "UnsetEnvironment=TEMP TMP", + "PassEnvironment=PATH", + "PassEnvironment=PATH HOME USER", + "EnvironmentFile=/etc/default/myservice", + "EnvironmentFile=-/etc/default/myservice", + + "SetCredential=username:nonsecret", + "SetCredential=token1:abcdef123456 token2: token3:asdf:asdf", + "SetCredential=", + /* "-EPIPE SetCredentialEncrypted=password:encrypted_data token:encrypted_too", */ + + "LoadCredential=", + "LoadCredential=asdf", + "LoadCredential=cert:/etc/ssl/cert.pem", + "LoadCredential=key:/etc/ssl/key.pem", + "LoadCredentialEncrypted=password:encrypted_file", + "ImportCredential=*", + "ImportCredential=prefix.*", + "ImportCredentialEx=*", + "ImportCredentialEx=prefix.*", + + "LogExtraFields=FIELD1=value1", + "LogExtraFields=FIELD1=value1 FIELD2=value2", + "LogFilterPatterns=~debug", + "LogFilterPatterns=~debug ~trace", + + "StandardInput=null", + "StandardInput=tty", + "StandardInput=socket", + "StandardOutput=journal", + "StandardOutput=syslog", + "StandardOutput=null", + "StandardError=journal", + "StandardError=syslog", + "StandardError=null", + "StandardInputText=Hello World", + "StandardInputText=Multi\nLine\nText", + "StandardInputData=SGVsbG8gV29ybGQ=", + + "AppArmorProfile=myprofile", + "AppArmorProfile=unconfined", + "SmackProcessLabel=mylabel", + "SmackProcessLabel=_", + + "CapabilityBoundingSet=CAP_NET_BIND_SERVICE", + "CapabilityBoundingSet=CAP_NET_BIND_SERVICE CAP_SYS_TIME", + "AmbientCapabilities=CAP_NET_BIND_SERVICE", + "AmbientCapabilities=CAP_NET_BIND_SERVICE CAP_SYS_TIME", + + "CPUAffinity=0-3", + "CPUAffinity=0,2,4,6", + "CPUAffinity=1", + + "NUMAPolicy=default", + "NUMAPolicy=preferred", + "NUMAPolicy=bind", + "NUMAMask=0-3", + "NUMAMask=0,2,4,6", + + "RestrictAddressFamilies=AF_INET", + "RestrictAddressFamilies=AF_INET AF_INET6", + "RestrictAddressFamilies=~AF_NETLINK", + "RestrictFileSystems=ext4", + "RestrictFileSystems=ext4 xfs", + "RestrictFileSystems=~tmpfs", + "SystemCallFilter=@system-service", + "SystemCallFilter=read write open close", + "SystemCallFilter=~@debug", + "SystemCallLog=@system-service", + "SystemCallLog=read write open", + "RestrictNetworkInterfaces=lo", + "RestrictNetworkInterfaces=lo eth0", + "RestrictNetworkInterfaces=~wlan0", + "RestrictNamespaces=cgroup", + "RestrictNamespaces=cgroup ipc net", + "RestrictNamespaces=~user", + "DelegateNamespaces=pid", + "DelegateNamespaces=pid net", + + "BindPaths=/host/path:/container/path", + "BindPaths=/host/path:/container/path:rbind", + "BindReadOnlyPaths=/host/ro:/container/ro", + "BindReadOnlyPaths=/host/ro:/container/ro:rbind", + + "TemporaryFileSystem=/tmp", + "TemporaryFileSystem=/tmp:rw,nodev,nosuid,size=100M /with/colons/in/options:rw,size=10%:::::::: \"/with/spaces path/:option1:option2\"", + /* "-EINVAL TemporaryFileSystem=\"/tmp path", */ + "TemporaryFileSystem=", + + "RootHash=/a/path", + "RootHash=1234567890abcdef1234567890abcdef", + "RootHashSignature=/a/path", + "RootHashSignature=base64:zKFyIq7aZn4EpuCCmpcF9jPgD8JFE1g/xfT0Mas8X4M0WycyigRsQ4IH4yysufus0AORQsuk3oeGhRC7t1tLyKD0Ih0VcYedv5+p8e6itqrIwzecu98+rNyUVDhWBzS0PMwxEw==", + + "RootImageOptions=partition=root,rw", + "RootImageOptions=partition=usr,ro", + "MountImages=/path/to/image.raw:/mount/point", + "MountImages=/path/to/image.raw:/mount/point:partition=1", + "ExtensionImages=/path/to/ext.raw", + "ExtensionImages=/path/to/ext.raw:/opt/extension", + + "StateDirectory=myapp", + "StateDirectory=myapp subdir", + "RuntimeDirectory=myapp", + "RuntimeDirectory=myapp subdir", + "CacheDirectory=myapp", + "CacheDirectory=myapp subdir", + "LogsDirectory=myapp", + "LogsDirectory=myapp subdir", + + "ProtectHostname=true", + "ProtectHostname=false", + "ProtectHostname=private", + "ProtectHostnameEx=true", + "ProtectHostnameEx=false", + "ProtectHostnameEx=private", + "ProtectHostname=true:foo", + "ProtectHostname=false:foo", + "ProtectHostname=private:foo", + "ProtectHostnameEx=true:foo", + "ProtectHostnameEx=false:foo", + "ProtectHostnameEx=private:foo", + "ProtectHostname=private:a.b.c.d.example.org", + + "LimitCPU=15", + "LimitCPU=15:35", + "LimitCPU=infinity", + "LimitFSIZE=123", + "LimitDATA=1", + "LimitSTACK=1:2", + "LimitCORE=0", + "LimitRSS=0", + "LimitNPROC=0", + "LimitNOFILE=1234", + "LimitMEMLOCK=infinity", + "LimitAS=44", + "LimitLOCKS=22", + "LimitSIGPENDING=11", + "LimitMSGQUEUE=0", + "LimitNICE=0", + "LimitRTPRIO=0", + "LimitRTTIME=0" + ); + + test_transient_settings_one(UNIT_SOCKET, lines); + /* Also UNIT_SERVICE, UNIT_MOUNT. */ +} + +TEST(kill_properties) { + const char* const *lines = STRV_MAKE_CONST( + "KillMode=control-group", + "KillMode=process", + "KillMode=mixed", + "KillMode=none", + + "SendSIGHUP=true", + "SendSIGKILL=true", + + "KillSignal=1", + "KillSignal=64", /* _NSIG == 64 */ + "-EINVAL KillSignal=0", + "-EINVAL KillSignal=65", + "RestartKillSignal=TERM", + "RestartKillSignal=SIGTERM", + "FinalKillSignal=WINCH", + "FinalKillSignal=SIGWINCH", + "FinalKillSignal=2", + "WatchdogSignal=RTMIN", + "WatchdogSignal=RTMIN+0", + "ReloadSignal=RTMAX", + "ReloadSignal=RTMAX-0", + "ReloadSignal=RTMAX-5", + "-EINVAL ReloadSignal=RTMAX-100" + ); + + test_transient_settings_one(UNIT_SCOPE, lines); + /* Also UNIT_SERVICE, UNIT_SOCKET, UNIT_MOUNT. */ +} + +TEST(mount_properties) { + const char* const *lines = STRV_MAKE_CONST( + "What=/dev/sda1", + "What=UUID=12345678-1234-1234-1234-123456789012", + + "Where=/mnt/disk", + "Where=non/absolute/var/lib/data", // TODO: should this be allowed? + + "Options=defaults", + "Options= rw,noatime,user_xattr,acl more ", + "Options=", + + "Type=ext4", + "TimeoutSec= 90 s ", + "DirectoryMode=0755", + "SloppyOptions=true", + "LazyUnmount=true", + "ForceUnmount=true", + "ReadwriteOnly=true" + ); + + test_transient_settings_one(UNIT_MOUNT, lines); +} + +TEST(path_properties) { + const char* const *lines = STRV_MAKE_CONST( + "MakeDirectory=true", + "DirectoryMode=0", + "DirectoryMode=1", + + "PathExists=/var/lib/myapp/ready", + "PathExists=/tmp/../././././././././.../.../.../.../.../.../lockfile", + + "PathExistsGlob=/var/log/*.log", + "PathExistsGlob=/home/*/Desktop???", + + "PathChanged=/etc/myapp.conf", + "PathModified=/etc/passwd", + "DirectoryNotEmpty=/var/spool/mail", + "TriggerLimitBurst=10", + "PollLimitBurst=100", + "TriggerLimitIntervalSec=2s", + "PollLimitIntervalSec=5s" + ); + + test_transient_settings_one(UNIT_PATH, lines); +} + +TEST(scope_properties) { + const char* const *lines = STRV_MAKE_CONST( + "RuntimeMaxSec=3600s", + "RuntimeRandomizedExtraSec=60s", + "TimeoutStopSec=90s", + "OOMPolicy=stop", + "OOMPolicy=continue", + "User=_some_user", + "User=1000", + "Group=mygroup", + "Group=users" + ); + + test_transient_settings_one(UNIT_SCOPE, lines); +} + +TEST(service_properties) { + const char* const *lines = STRV_MAKE_CONST( + /* String properties — no validity checking */ + "PIDFile=/var/run/myservice.pid", + "PIDFile=/run/myservice.pid", + "PIDFile=/tmp/daemon.pid", + "Type=simple", + "Type=forking", + "Type=oneshot", + "Type=dbus", + "Type=notify", + "Type=idle", + "Type=exec", + "Type= asdf ", + "ExitType=main", + "ExitType=cgroup", + "Restart=no", + "Restart=on-success", + "Restart=on-failure", + "Restart=on-abnormal", + "Restart=on-watchdog", + "Restart=on-abort", + "Restart=always", + "RestartMode=normal", + "RestartMode=direct", + "BusName=org.example.MyService", + "BusName=com.company.daemon", + "BusName=net.domain.service", + "NotifyAccess=none", + "NotifyAccess=main", + "NotifyAccess=exec", + "NotifyAccess=all", + "USBFunctionDescriptors=/dev/usb-ffs/myfunction", + "USBFunctionStrings=/dev/usb-ffs/myfunction", + "OOMPolicy=stop", + "OOMPolicy=continue", + "OOMPolicy=kill", + "TimeoutStartFailureMode=terminate", + "TimeoutStartFailureMode=abort", + "TimeoutStartFailureMode=kill", + "TimeoutStopFailureMode=terminate", + "TimeoutStopFailureMode=abort", + "TimeoutStopFailureMode=kill", + "FileDescriptorStorePreserve=no", + "FileDescriptorStorePreserve=yes", + "FileDescriptorStorePreserve=restart", + + /* Boolean properties */ + "PermissionsStartOnly=true", + "RootDirectoryStartOnly=true", + "RemainAfterExit=true", + "GuessMainPID=true", + "-EINVAL GuessMainPID=WAT", + + /* Timespan properties */ + "RestartSec=5s", + "RestartSec=1 y 2 months 30s", + "RestartMaxDelaySec=5min", + "TimeoutStartSec=infinity", + "TimeoutStopSec=infinity", + "TimeoutAbortSec=0", + "RuntimeMaxSec=24h", + "RuntimeRandomizedExtraSec=30s", + "WatchdogSec=30s", + "TimeoutSec=90s", + + /* Unsigned integer values */ + "FileDescriptorStoreMax=0", + "FileDescriptorStoreMax=9999999", + "RestartSteps=10", + + /* Exec command properties */ + "ExecCondition=test -f /etc/myservice.conf", + "ExecStartPre=/usr/bin/mkdir -p /var/lib/myservice", + "ExecStartPre=-/usr/bin/systemctl is-active network.target", + "ExecStartPre=@/usr/bin/systemctl is-active network.target", + "ExecStartPre=:systemctl is-active network.target", + "ExecStartPre=+systemctl is-active network.target", + "ExecStartPre=!systemctl is-active network.target", + "ExecStartPre=|systemctl is-active network.target", + "ExecStartPre=!!true", + "ExecStart=@:+!|foobar bar bar", + "ExecStart=@:+!!|foobar bar bar", + "ExecStartPost=post post" + "ExecConditionEx=/usr/bin/test -f /etc/myservice.conf", + "ExecStartPreEx=/usr/bin/mkdir -p /var/lib/myservice", + "ExecStartEx=/usr/bin/myservice --config /etc/myservice.conf", + "ExecStartPostEx=/usr/bin/logger 'Service started'", + "ExecReload=/bin/kill -HUP $MAINPID", + "ExecStop=kill -TERM $MAINPID", + "ExecStopPost=+logger 'Service stopped'", + "ExecStopPost=rm -f /var/lib/myservice/started", + "ExecReloadEx=/bin/kill -HUP $MAINPID", + "ExecStopEx=/bin/kill -TERM $MAINPID", + "ExecStopPostEx=/usr/bin/logger 'Service stopped'", + + "RestartPreventExitStatus=1", + "RestartPreventExitStatus=1 2 8 SIGTERM", + "RestartPreventExitStatus=SIGKILL SIGTERM", + "RestartForceExitStatus=1", + "RestartForceExitStatus=1 2 8 SIGTERM", + "RestartForceExitStatus=SIGKILL SIGTERM", + "SuccessExitStatus=0", + "SuccessExitStatus=0 1 2", + "SuccessExitStatus=0 SIGTERM SIGKILL", + + "OpenFile=/a/path", + "OpenFile=/etc/myservice.conf:SOME$NAME", + "OpenFile=/etc/myservice.conf:SOME$NAME:read-only", + "OpenFile=/etc/myservice.conf:SOME$NAME:append", + "OpenFile=/etc/myservice.conf:SOME$NAME:truncate", + "OpenFile=/etc/myservice.conf:SOME$NAME:graceful", + "OpenFile=/etc/myservice.conf::read-only,graceful", + "OpenFile=/etc/myservice.conf::truncate,graceful", + "-EINVAL OpenFile=/etc/myservice.conf::append,truncate,read-only,graceful" + ); + + test_transient_settings_one(UNIT_SERVICE, lines); +} + +TEST(socket_properties) { + const char* const *lines = STRV_MAKE_CONST( + /* Boolean properties */ + "Accept=true", + "FlushPending=true", + "Writable=true", + "KeepAlive=true", + "NoDelay=true", + "FreeBind=true", + "Transparent=true", + "Broadcast=true", + "PassCredentials=true", + "PassFileDescriptorsToExec=true", + "PassSecurity=true", + "PassPacketInfo=true", + "ReusePort=true", + "RemoveOnStop=true", + "SELinuxContextFromNet=true", + + /* Integer properties */ + "Priority=6", + "Priority=0000000", + "Priority=-1", + "IPTTL=64", + "IPTTL=255", + "IPTTL=1", + "Mark=1", + "Mark=0", + "Mark=255", + + /* IP TOS properties */ + "IPTOS=0", + "IPTOS=low-delay", + "IPTOS=throughput", + "IPTOS=reliability", + "IPTOS=low-cost", + + /* Unsigned integer properties */ + "Backlog=0", + "Backlog=128", + "MaxConnections=64", + "MaxConnectionsPerSource=10", + "KeepAliveProbes=9", + "TriggerLimitBurst=10", + "PollLimitBurst=10", + + /* Mode properties */ + "SocketMode=0666", + "SocketMode=644", + "DirectoryMode=0755", + "DirectoryMode=0", + + /* 64-bit integer properties */ + "MessageQueueMaxMessages=10", + "MessageQueueMessageSize=8192", + + /* Timespan properties */ + "TimeoutSec=90s", + "TimeoutSec=5min", + "TimeoutSec=infinity", + "KeepAliveTimeSec=7200s", + "KeepAliveTimeSec=2h", + "KeepAliveTimeSec=30min", + "KeepAliveIntervalSec=75s", + "KeepAliveIntervalSec=1min", + "KeepAliveIntervalSec=30s", + "DeferAcceptSec=1w", + "TriggerLimitIntervalSec=2s", + "PollLimitIntervalSec=2s", + "DeferTriggerMaxSec=infinity", + + /* Size properties */ + "ReceiveBuffer=262144", + "ReceiveBuffer=1M", + "ReceiveBuffer=512K", + "SendBuffer=1.M", // TODO: should this accept multiple components? + "PipeSize=512K", + + /* Exec command properties */ + "ExecStartPre=true", + "ExecStartPost=false", + "ExecReload=kill", + "ExecStopPost=signal", + + /* More string properties */ + "SmackLabel=mylabel", + "SmackLabel=_", + "SmackLabelIPIn=mylabel", + "SmackLabelIPIn=_", + "SmackLabelIPOut=mylabel", + "SmackLabelIPOut=_", + "TCPCongestion=cubic", + "TCPCongestion=reno", + "TCPCongestion=bbr", + "BindToDevice=eth0", + "BindToDevice=wlan0", + "BindToDevice=lo", + "BindIPv6Only=default", + "BindIPv6Only=both", + "BindIPv6Only=ipv6-only", + "FileDescriptorName=myfd", + "FileDescriptorName=socket", + "FileDescriptorName=listener", + "SocketUser=myuser", + "SocketUser=www-data", + "SocketGroup=mygroup", + "SocketGroup=www-data", + "Timestamping=off", + "Timestamping=us", + "Timestamping=ns", + "DeferTrigger=on", + "DeferTrigger=off", + "DeferTrigger=all", + + /* Path strv */ + "Symlinks=/var/lib/socket/link", + "Symlinks=/var/lib/socket/link /tmp/socket-link non/abs/path", + + /* Socket protocol properties */ + "SocketProtocol=udp", + "SocketProtocol=tcp", + "SocketProtocol=sctp", + + /* Listen* properties */ + "ListenStream=8080", + "ListenStream=127.0.0.1:8080", + "ListenStream=[::1]:8080", + "ListenStream=/run/myservice.sock", + "ListenDatagram=8080", + "ListenDatagram=127.0.0.1:8080", + "ListenDatagram=[::1]:8080", + "ListenDatagram=/run/myservice.sock", + "ListenSequentialPacket=/run/myservice.sock", + "ListenSequentialPacket=/tmp/socket", + "ListenNetlink=kobject-uevent", + "ListenNetlink=audit", + "ListenNetlink=route", + "ListenSpecial=/dev/log", + "ListenSpecial=/dev/kmsg", + "ListenSpecial=/proc/kmsg", + "ListenMessageQueue=/myqueue", + "ListenMessageQueue=/system/queue", + "ListenFIFO=/var/lib/myservice/fifo", + "ListenFIFO=/tmp/myfifo", + "ListenUSBFunction=/dev/usb-ffs/myfunction" + ); + test_transient_settings_one(UNIT_SOCKET, lines); +} + +TEST(timer_properties) { + const char* const *lines = STRV_MAKE_CONST( + "WakeSystem=true", + "RemainAfterElapse=yes", + "Persistent=true", + "OnTimezoneChange=yes", + "OnClockChange=true", + "FixedRandomDelay=yes", + "DeferReactivation=true", + + "AccuracySec=1s", + "AccuracySec=10min", + "AccuracySec=1h", + "RandomizedDelaySec=30s", + "RandomizedDelaySec=5min", + "RandomizedDelaySec=0", + "RandomizedOffsetSec=15s", + "RandomizedOffsetSec=2min", + "OnActiveSec=10s", + "OnActiveSec=5min", + "OnBootSec=30s", + "OnBootSec=2min", + "OnStartupSec=45s", + "OnStartupSec=1min", + "OnUnitActiveSec=20s", + "OnUnitActiveSec=10min", + "OnUnitInactiveSec=60s", + "OnUnitInactiveSec=30min", + "OnCalendar=daily", + "OnCalendar=Mon,Tue *-*-* 12:00:00", + "OnCalendar=*-*-* 06:00:00", + "OnCalendar=Sat *-*-* 23:00:00" + ); + + test_transient_settings_one(UNIT_TIMER, lines); +} + +TEST(unit_properties) { + const char* const *lines = STRV_MAKE_CONST( + "Description=My Service Unit", + "SourcePath=/etc/systemd/system/myservice.service", + "SourcePath=non/abs/lib/systemd/system/backup.service", + "OnFailureJobMode=replace", + "OnFailureJobMode=fail", + "OnFailureJobMode=isolate", + "JobTimeoutAction=none", + "JobTimeoutAction=reboot", + "JobTimeoutAction=poweroff", + "JobTimeoutRebootArgument=emergency", + "JobTimeoutRebootArgument=--force", + "StartLimitAction=none", + "StartLimitAction=reboot", + "StartLimitAction=reboot-force", + "FailureAction=none", + "FailureAction=restart", + "FailureAction=reboot", + "SuccessAction=none", + "SuccessAction=poweroff", + "SuccessAction=exit", + "RebootArgument=--force", + "RebootArgument=emergency", + "CollectMode=inactive", + "CollectMode=inactive-or-failed", + "StopWhenUnneeded=true", + "StopWhenUnneeded=false", + "RefuseManualStart=yes", + "RefuseManualStart=no", + "RefuseManualStop=true", + "RefuseManualStop=false", + "AllowIsolate=yes", + "AllowIsolate=no", + "IgnoreOnIsolate=true", + "IgnoreOnIsolate=false", + "SurviveFinalKillSignal=yes", + "SurviveFinalKillSignal=no", + "DefaultDependencies=true", + "DefaultDependencies=false", + "JobTimeoutSec=90s", + "JobTimeoutSec=5min", + "JobTimeoutSec=infinity", + "JobRunningTimeoutSec=30min", + "JobRunningTimeoutSec=1h", + "StartLimitIntervalSec=10s", + "StartLimitIntervalSec=60s", + "StartLimitBurst=5", + "StartLimitBurst=9999999", + "StartLimitBurst=0", + "SuccessActionExitStatus=0", + "SuccessActionExitStatus=", + "FailureActionExitStatus=1", + "FailureActionExitStatus=", + "Documentation=man:myservice(8)", + "Documentation=https://example.com/docs", + "Documentation=file:/usr/share/doc/myservice/README", + "RequiresMountsFor=/var/lib/myservice", + "RequiresMountsFor=\t\t\t \t/home\t\t\t\t\t/var/log \t", + "WantsMountsFor=/tmp", + "WantsMountsFor=/opt /usr/local", + "Markers=manual", + "Markers=needs-reload", + "Markers=manual needs-reload", + + + "Requires=a.service b.service", + "Requisite=a.service b.service", + "Wants=a.service b.service", + "BindsTo=a.service b.service", + "PartOf=a.service b.service", + "Upholds=a.service b.service", + "RequiredBy=a.service b.service", + "RequisiteOf=a.service b.service", + "WantedBy=a.service b.service", + "BoundBy=a.service b.service", + "ConsistsOf=a.service b.service", + "UpheldBy=a.service b.service", + "Conflicts=a.service b.service", + "ConflictedBy=a.service b.service", + "Before=a.service b.service", + "After=a.service b.service", + "OnSuccess=a.service b.service", + "OnSuccessOf=a.service b.service", + "OnFailure=a.service b.service", + "OnFailureOf=a.service b.service", + "Triggers=a.service b.service", + "TriggeredBy=a.service b.service", + "PropagatesReloadTo=a.service b.service", + "ReloadPropagatedFrom=a.service b.service", + "PropagatesStopTo=a.service b.service", + "StopPropagatedFrom=a.service b.service", + "JoinsNamespaceOf=a.service b.service", + "References=a.service b.service", + "ReferencedBy=a.service b.service", + "InSlice=a.service b.service", + "SliceOf=a.service b.service", + + // TODO: do better verification here? + "ConditionArchitecture=aarch64", + "ConditionArchitecture=|!aarch64", + "ConditionArchitecture=!aarch64", + "ConditionArchitecture=|aarch64", + "ConditionArchitecture=|!", + "ConditionFirmware=something", + "ConditionVirtualization=|foo", + "ConditionHost=|foo", + "ConditionKernelCommandLine=|foo", + "ConditionVersion=|foo", + "ConditionCredential=|foo", + "ConditionSecurity=|foo", + "ConditionCapability=|foo", + "ConditionACPower=|foo", + "ConditionMemory=|foo", + "ConditionCPUs=|foo", + "ConditionEnvironment=|foo", + "ConditionCPUFeature=|foo", + "ConditionOSRelease=|foo", + "ConditionMemoryPressure=|foo", + "ConditionCPUPressure=|foo", + "ConditionIOPressure=|foo", + "ConditionNeedsUpdate=|foo", + "ConditionFirstBoot=|foo", + "ConditionPathExists=|foo", + "ConditionPathExistsGlob=|foo", + "ConditionPathIsDirectory=|foo", + "ConditionPathIsSymbolicLink=|foo", + "ConditionPathIsMountPoint=|foo", + "ConditionPathIsReadWrite=|foo", + "ConditionPathIsEncrypted=|foo", + "ConditionDirectoryNotEmpty=|foo", + "ConditionFileNotEmpty=|foo", + "ConditionFileIsExecutable=|foo", + "ConditionUser=|foo", + "ConditionGroup=|foo", + "ConditionControlGroupController=|foo", + "ConditionKernelModuleLoaded=|foo", + "AssertArchitecture=|foo", + "AssertFirmware=|foo", + "AssertVirtualization=|foo", + "AssertHost=|foo", + "AssertKernelCommandLine=|foo", + "AssertVersion=|foo", + "AssertCredential=|foo", + "AssertSecurity=|foo", + "AssertCapability=|foo", + "AssertACPower=|foo", + "AssertMemory=|foo", + "AssertCPUs=|foo", + "AssertEnvironment=|foo", + "AssertCPUFeature=|foo", + "AssertOSRelease=|foo", + "AssertMemoryPressure=|foo", + "AssertCPUPressure=|foo", + "AssertIOPressure=|foo", + "AssertNeedsUpdate=|foo", + "AssertFirstBoot=|foo", + "AssertPathExists=|foo", + "AssertPathExistsGlob=|foo", + "AssertPathIsDirectory=|foo", + "AssertPathIsSymbolicLink=|foo", + "AssertPathIsMountPoint=|foo", + "AssertPathIsReadWrite=|foo", + "AssertPathIsEncrypted=|foo", + "AssertDirectoryNotEmpty=|foo", + "AssertFileNotEmpty=|foo", + "AssertFileIsExecutable=|foo", + "AssertUser=|foo", + "AssertGroup=|foo", + "AssertControlGroupController=|foo", + "AssertKernelModuleLoaded=|foo" + ); + + test_transient_settings_one(UNIT_TARGET, lines); + /* All unit types. */ +} + TEST(bus_dump_transient_settings) { - /* -1 is for generic unit, natural numbers are for specific unit types */ for (UnitType t = 0; t < _UNIT_TYPE_MAX; t++) { log_info("==================== %s ====================", t < 0 ? "unit" : unit_type_to_string(t)); bus_dump_transient_settings(t); } } -DEFINE_TEST_MAIN(LOG_DEBUG); +static int intro(void) { + int r; + + r = sd_bus_default_user(&arg_bus); + if (r < 0) + r = sd_bus_default_system(&arg_bus); + if (r < 0) + log_info_errno(r, "Failed to connect to bus: %m"); + + return EXIT_SUCCESS; +} + +DEFINE_TEST_MAIN_WITH_INTRO(LOG_DEBUG, intro); From 8d853534edb186b079030adb5f3c602ef58fee41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Jul 2025 12:07:13 +0200 Subject: [PATCH 3/9] shared/bus-unit-util: also send empty array for LogFilterPatterns= Before, for empty input, we'd send an array with one item with an empty pattern. Use the helper which sends an empty array instead. bus_exec_context_set_transient_property() ignores items with an empty pattern, so the result should be the same. Request in review: https://github.com/systemd/systemd/pull/37665#discussion_r2182375988. --- src/shared/bus-unit-util.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 9a4a8a865a..e053cd31a6 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -1223,15 +1223,10 @@ static int bus_append_log_extra_fields(sd_bus_message *m, const char *field, con } static int bus_append_log_filter_patterns(sd_bus_message *m, const char *field, const char *eq) { - int r; - - r = sd_bus_message_append(m, "(sv)", "LogFilterPatterns", "a(bs)", 1, - eq[0] != '~', - eq[0] != '~' ? eq : eq + 1); - if (r < 0) - return bus_log_create_error(r); - - return 1; + return bus_append_trivial_array(m, field, eq, + "a(bs)", + eq[0] != '~', + eq[0] != '~' ? eq : eq + 1); } static int bus_append_standard_inputs(sd_bus_message *m, const char *field, const char *eq) { From ce120ac82370a93409dcea4227deda73474038db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Jul 2025 14:06:52 +0200 Subject: [PATCH 4/9] shared/bus-unit-util: tweak error handling in bus_append_exec_command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit exec_command_flags_to_strv() should not fail, unless we screwed up, so assert instead of returning an error. Also, no need to strdup constant _PATH_BSHELL; drop that so that we can get rid of the oom error handling. Finally, rename l → cmdline for clarity. --- src/shared/bus-unit-util.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index e053cd31a6..d98550dd0b 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -561,8 +561,8 @@ static int bus_append_socket_filter(sd_bus_message *m, const char *field, const static int bus_append_exec_command(sd_bus_message *m, const char *field, const char *eq) { bool explicit_path = false, done = false, ambient_hack = false; - _cleanup_strv_free_ char **l = NULL, **ex_opts = NULL; - _cleanup_free_ char *path = NULL, *upgraded_name = NULL; + _cleanup_strv_free_ char **cmdline = NULL, **ex_opts = NULL; + _cleanup_free_ char *_path = NULL, *upgraded_name = NULL; ExecCommandFlags flags = 0; bool is_ex_prop = endswith(field, "Ex"); int r; @@ -655,27 +655,26 @@ static int bus_append_exec_command(sd_bus_message *m, const char *field, const c return log_error_errno(r, "Failed to convert ExecCommandFlags to strv: %m"); } - if (FLAGS_SET(flags, EXEC_COMMAND_VIA_SHELL)) { - path = strdup(_PATH_BSHELL); - if (!path) - return log_oom(); - - } else if (explicit_path) { - r = extract_first_word(&eq, &path, NULL, EXTRACT_UNQUOTE|EXTRACT_CUNESCAPE); + const char *path = NULL; + if (FLAGS_SET(flags, EXEC_COMMAND_VIA_SHELL)) + path = _PATH_BSHELL; + else if (explicit_path) { + r = extract_first_word(&eq, &_path, NULL, EXTRACT_UNQUOTE|EXTRACT_CUNESCAPE); if (r < 0) return log_error_errno(r, "Failed to parse path: %m"); if (r == 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No executable path specified, refusing."); if (isempty(eq)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Got empty command line, refusing."); + path = _path; } - r = strv_split_full(&l, eq, NULL, EXTRACT_UNQUOTE|EXTRACT_CUNESCAPE); + r = strv_split_full(&cmdline, eq, NULL, EXTRACT_UNQUOTE|EXTRACT_CUNESCAPE); if (r < 0) return log_error_errno(r, "Failed to parse command line: %m"); if (FLAGS_SET(flags, EXEC_COMMAND_VIA_SHELL)) { - r = strv_prepend(&l, explicit_path ? "-sh" : "sh"); + r = strv_prepend(&cmdline, explicit_path ? "-sh" : "sh"); if (r < 0) return log_oom(); } @@ -696,21 +695,21 @@ static int bus_append_exec_command(sd_bus_message *m, const char *field, const c if (r < 0) return bus_log_create_error(r); - if (!strv_isempty(l)) { - + if (!strv_isempty(cmdline)) { r = sd_bus_message_open_container(m, 'r', is_ex_prop ? "sasas" : "sasb"); if (r < 0) return bus_log_create_error(r); - r = sd_bus_message_append(m, "s", path ?: l[0]); + r = sd_bus_message_append(m, "s", path ?: cmdline[0]); if (r < 0) return bus_log_create_error(r); - r = sd_bus_message_append_strv(m, l); + r = sd_bus_message_append_strv(m, cmdline); if (r < 0) return bus_log_create_error(r); - r = is_ex_prop ? sd_bus_message_append_strv(m, ex_opts) : sd_bus_message_append(m, "b", FLAGS_SET(flags, EXEC_COMMAND_IGNORE_FAILURE)); + r = is_ex_prop ? sd_bus_message_append_strv(m, ex_opts) : + sd_bus_message_append(m, "b", FLAGS_SET(flags, EXEC_COMMAND_IGNORE_FAILURE)); if (r < 0) return bus_log_create_error(r); From 95a3cfcbf6bedb211be0c7f9a7f6047b8748c435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Jul 2025 17:18:16 +0200 Subject: [PATCH 5/9] shared/bus-unit-util: rework error messages We generally want to have error messages with a fixed structure that convey the important information, i.e. field name, error value, and the offending text for options that take short values. (The text is not printed for strings encoded with base64 and hexmem or for credentials.) Let's use a helper that prints the message in a fixed format in the majority of cases. In the few places where a custom message is useful, the helper is not used. The helper: - prints the field name, value, and error info, - quotes the value, - handles -ENOMEM, so we don't need to handle it separately everywhere. When this code was originally written, parse functions would return -1 as error. Nowadays day all return a good errno, so it is fine if we print the corresponding strerror. --- src/shared/bus-unit-util.c | 191 +++++++++++++++------------------- src/test/test-bus-unit-util.c | 8 +- 2 files changed, 87 insertions(+), 112 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index d98550dd0b..ebfabcdf36 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -76,6 +76,17 @@ static int warn_deprecated(_unused_ sd_bus_message *m, const char *field, const return 1; } +static int parse_log_error(int error, const char *field, const char *eq) { + if (error == -ENOMEM) + return log_oom(); + if (error != 0) /* Allow SYNTHETIC_ERRNO to be used, i.e. positive values. */ + return log_error_errno(error, "Failed to parse %s= value '%s': %m", field, eq); + + /* We don't log the error value for cases where we have a general "syntax error". */ + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Invalid syntax for %s= value: '%s'", field, eq); +} + #define DEFINE_BUS_APPEND_PARSE_PTR(bus_type, cast_type, type, parse_func) \ static int bus_append_##parse_func( \ sd_bus_message *m, \ @@ -86,7 +97,7 @@ static int warn_deprecated(_unused_ sd_bus_message *m, const char *field, const \ r = parse_func(eq, &val); \ if (r < 0) \ - return log_error_errno(r, "Failed to parse %s=%s: %m", field, eq); \ + return parse_log_error(r, field, eq); \ \ r = sd_bus_message_append(m, "(sv)", field, \ bus_type, (cast_type) val); \ @@ -105,7 +116,7 @@ static int warn_deprecated(_unused_ sd_bus_message *m, const char *field, const \ r = parse_func(eq); \ if (r < 0) \ - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to parse %s: %s", field, eq); \ + return parse_log_error(r, field, eq); \ \ r = sd_bus_message_append(m, "(sv)", field, \ bus_type, (int32_t) r); \ @@ -175,10 +186,8 @@ static int bus_append_strv_full(sd_bus_message *m, const char *field, const char _cleanup_free_ char *word = NULL; r = extract_first_word(&p, &word, /* separators= */ NULL, flags); - if (r == -ENOMEM) - return log_oom(); if (r < 0) - return log_error_errno(r, "Invalid syntax: %s", eq); + return parse_log_error(r, field, eq); if (r == 0) break; @@ -248,7 +257,7 @@ static int bus_append_parse_sec_rename(sd_bus_message *m, const char *field, con r = parse_sec(eq, &t); if (r < 0) - return log_error_errno(r, "Failed to parse %s=%s: %m", field, eq); + return parse_log_error(r, field, eq); l = strlen(field); n = newa(char, l + 2); @@ -272,7 +281,7 @@ static int bus_append_parse_size(sd_bus_message *m, const char *field, const cha r = parse_size(eq, /* base= */ 1024, &v); if (r < 0) - return log_error_errno(r, "Failed to parse %s=%s: %m", field, eq); + return parse_log_error(r, field, eq); r = sd_bus_message_append(m, "(sv)", field, "t", v); if (r < 0) @@ -286,7 +295,7 @@ static int bus_append_parse_permyriad(sd_bus_message *m, const char *field, cons r = parse_permyriad(eq); if (r < 0) - return log_error_errno(r, "Failed to parse %s=%s: %m", field, eq); + return parse_log_error(r, field, eq); /* Pass around scaled to 2^32-1 == 100% */ r = sd_bus_message_append(m, "(sv)", field, "u", UINT32_SCALE_FROM_PERMYRIAD(r)); @@ -304,7 +313,7 @@ static int bus_append_parse_cpu_set(sd_bus_message *m, const char *field, const r = parse_cpu_set(eq, &cpuset); if (r < 0) - return log_error_errno(r, "Failed to parse %s value: %s", field, eq); + return parse_log_error(r, field, eq); r = cpu_set_to_dbus(&cpuset, &array, &allocated); if (r < 0) @@ -376,9 +385,9 @@ static int bus_append_parse_cpu_quota(sd_bus_message *m, const char *field, cons else { r = parse_permyriad_unbounded(eq); if (r < 0) - return log_error_errno(r, "Failed to parse %s=%s: %m", field, eq); + return parse_log_error(r, field, eq); if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(ERANGE), "%s value too small.", field); + return parse_log_error(SYNTHETIC_ERRNO(ERANGE), field, eq); x = r * USEC_PER_SEC / 10000U; } @@ -426,9 +435,7 @@ static int bus_try_append_parse_cgroup_io_limit(sd_bus_message *m, const char *f else { const char *e = strchr(eq, ' '); if (!e) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Failed to parse %s value %s.", - field, eq); + return parse_log_error(0, field, eq); const char *bandwidth = e + 1; _cleanup_free_ char *path = strndup(eq, e - eq); @@ -441,7 +448,7 @@ static int bus_try_append_parse_cgroup_io_limit(sd_bus_message *m, const char *f else { r = parse_size(bandwidth, 1000, &bytes); if (r < 0) - return log_error_errno(r, "Failed to parse byte value %s: %m", bandwidth); + return parse_log_error(r, field, eq); } r = sd_bus_message_append(m, "(sv)", field, "a(st)", 1, path, bytes); @@ -460,9 +467,7 @@ static int bus_append_parse_io_device_weight(sd_bus_message *m, const char *fiel else { const char *e = strchr(eq, ' '); if (!e) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Failed to parse %s value %s.", - field, eq); + return parse_log_error(0, field, eq); const char *weight = e + 1; _cleanup_free_ char *path = strndup(eq, e - eq); @@ -472,7 +477,7 @@ static int bus_append_parse_io_device_weight(sd_bus_message *m, const char *fiel uint64_t u; r = safe_atou64(weight, &u); if (r < 0) - return log_error_errno(r, "Failed to parse %s value %s: %m", field, weight); + return parse_log_error(r, field, weight); r = sd_bus_message_append(m, "(sv)", field, "a(st)", 1, path, u); } @@ -491,9 +496,7 @@ static int bus_append_parse_io_device_latency(sd_bus_message *m, const char *fie else { const char *e = strchr(eq, ' '); if (!e) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Failed to parse %s value %s.", - field, eq); + return parse_log_error(0, field, eq); const char *target = e + 1; _cleanup_free_ char *path = strndup(eq, e - eq); @@ -503,7 +506,7 @@ static int bus_append_parse_io_device_latency(sd_bus_message *m, const char *fie usec_t usec; r = parse_sec(target, &usec); if (r < 0) - return log_error_errno(r, "Failed to parse %s value %s: %m", field, target); + return parse_log_error(r, field, target); r = sd_bus_message_append(m, "(sv)", field_usec, "a(st)", 1, path, usec); } @@ -522,10 +525,8 @@ static int bus_append_bpf_program(sd_bus_message *m, const char *field, const ch _cleanup_free_ char *word = NULL; r = extract_first_word(&eq, &word, ":", 0); - if (r == -ENOMEM) - return log_oom(); if (r < 0) - return log_error_errno(r, "Failed to parse %s: %m", field); + return parse_log_error(r, field, eq); r = sd_bus_message_append(m, "(sv)", field, "a(ss)", 1, word, eq); } @@ -545,10 +546,8 @@ static int bus_append_socket_filter(sd_bus_message *m, const char *field, const uint16_t nr_ports, port_min; r = parse_socket_bind_item(eq, &family, &ip_protocol, &nr_ports, &port_min); - if (r == -ENOMEM) - return log_oom(); if (r < 0) - return log_error_errno(r, "Failed to parse %s", field); + return parse_log_error(r, field, eq); r = sd_bus_message_append( m, "(sv)", field, "a(iiqq)", 1, family, ip_protocol, nr_ports, port_min); @@ -652,7 +651,7 @@ static int bus_append_exec_command(sd_bus_message *m, const char *field, const c if (is_ex_prop) { r = exec_command_flags_to_strv(flags, &ex_opts); if (r < 0) - return log_error_errno(r, "Failed to convert ExecCommandFlags to strv: %m"); + return log_error_errno(r, "Failed to serialize ExecCommand flags: %m"); } const char *path = NULL; @@ -661,17 +660,17 @@ static int bus_append_exec_command(sd_bus_message *m, const char *field, const c else if (explicit_path) { r = extract_first_word(&eq, &_path, NULL, EXTRACT_UNQUOTE|EXTRACT_CUNESCAPE); if (r < 0) - return log_error_errno(r, "Failed to parse path: %m"); + return parse_log_error(r, field, eq); if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No executable path specified, refusing."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No executable path specified for %s=, refusing.", field); if (isempty(eq)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Got empty command line, refusing."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Got empty command line for %s=, refusing.", field); path = _path; } r = strv_split_full(&cmdline, eq, NULL, EXTRACT_UNQUOTE|EXTRACT_CUNESCAPE); if (r < 0) - return log_error_errno(r, "Failed to parse command line: %m"); + return parse_log_error(r, field, eq); if (FLAGS_SET(flags, EXEC_COMMAND_VIA_SHELL)) { r = strv_prepend(&cmdline, explicit_path ? "-sh" : "sh"); @@ -741,7 +740,7 @@ static int bus_append_open_file(sd_bus_message *m, const char *field, const char r = open_file_parse(eq, &of); if (r < 0) - return log_error_errno(r, "Failed to parse OpenFile= setting: %m"); + return parse_log_error(r, field, eq); r = sd_bus_message_append(m, "(sv)", field, "a(sst)", (size_t) 1, of->path, of->fdname, of->flags); if (r < 0) @@ -863,16 +862,14 @@ static int bus_append_parse_ip_address_filter(sd_bus_message *m, const char *fie _cleanup_free_ char *word = NULL; r = extract_first_word(&eq, &word, NULL, 0); - if (r == -ENOMEM) - return log_oom(); if (r < 0) - return log_error_errno(r, "Failed to parse %s: %s", field, eq); + return parse_log_error(r, field, eq); if (r == 0) break; r = in_addr_prefix_from_string_auto(word, &family, &prefix, &prefixlen); if (r < 0) - return log_error_errno(r, "Failed to parse IP address prefix: %s", word); + return log_error_errno(r, "Failed to parse IP address prefix '%s': %m", word); r = bus_append_ip_address_access(m, family, &prefix, prefixlen); if (r < 0) @@ -947,21 +944,17 @@ static int bus_append_nft_set(sd_bus_message *m, const char *field, const char * int source, nfproto; r = extract_first_word(&p, &tuple, NULL, EXTRACT_UNQUOTE|EXTRACT_RETAIN_ESCAPE); - if (r == -ENOMEM) - return log_oom(); if (r < 0) - return log_error_errno(r, "Failed to parse %s: %m", field); + return parse_log_error(r, field, eq); if (r == 0) break; if (isempty(tuple)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to parse %s.", field); + return parse_log_error(0, field, eq); q = tuple; r = extract_many_words(&q, ":", EXTRACT_CUNESCAPE, &source_str, &nfproto_str, &table, &set); - if (r == -ENOMEM) - return log_oom(); if (r != 4 || !isempty(q)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to parse %s.", field); + return parse_log_error(0, field, tuple); assert(source_str); assert(nfproto_str); @@ -1024,12 +1017,8 @@ static int bus_append_set_credential(sd_bus_message *m, const char *field, const const char *p = eq; r = extract_first_word(&p, &word, ":", EXTRACT_DONT_COALESCE_SEPARATORS); - if (r == -ENOMEM) - return log_oom(); - if (r < 0) - return log_error_errno(r, "Failed to parse %s= parameter: %s", field, eq); - if (r == 0 || !p) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Missing argument to %s=.", field); + if (r <= 0 || !p) + return parse_log_error(r < 0 ? r : 0, field, eq); r = sd_bus_message_open_container(m, 'a', "(say)"); if (r < 0) @@ -1043,13 +1032,13 @@ static int bus_append_set_credential(sd_bus_message *m, const char *field, const if (r < 0) return bus_log_create_error(r); - if (streq(field, "SetCredentialEncrypted")) { + if (endswith(field, "Encrypted")) { _cleanup_free_ void *decoded = NULL; size_t decoded_size; r = unbase64mem(p, &decoded, &decoded_size); if (r < 0) - return log_error_errno(r, "Failed to base64 decode encrypted credential: %m"); + return log_error_errno(r, "Failed to decode base64 data for %s=: %m", field); r = sd_bus_message_append_array(m, 'y', decoded, decoded_size); } else { @@ -1058,7 +1047,7 @@ static int bus_append_set_credential(sd_bus_message *m, const char *field, const l = cunescape(p, UNESCAPE_ACCEPT_NUL, &unescaped); if (l < 0) - return log_error_errno(l, "Failed to unescape %s= value: %s", field, p); + return log_error_errno(l, "Failed to unescape value for %s=: %s", field, p); r = sd_bus_message_append_array(m, 'y', unescaped, l); } @@ -1107,12 +1096,8 @@ static int bus_append_load_credential(sd_bus_message *m, const char *field, cons const char *p = eq; r = extract_first_word(&p, &word, ":", EXTRACT_DONT_COALESCE_SEPARATORS); - if (r == -ENOMEM) - return log_oom(); - if (r < 0) - return log_error_errno(r, "Failed to parse %s= parameter: %s", field, eq); - if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Missing argument to %s=.", field); + if (r <= 0) + return parse_log_error(r, field, eq); if (isempty(p)) /* If only one field is specified, then this means "inherit from above" */ p = eq; @@ -1143,12 +1128,8 @@ static int bus_append_import_credential(sd_bus_message *m, const char *field, co const char *p = eq; r = extract_first_word(&p, &word, ":", EXTRACT_DONT_COALESCE_SEPARATORS); - if (r == -ENOMEM) - return log_oom(); - if (r < 0) - return log_error_errno(r, "Failed to parse %s= parameter: %s", field, eq); - if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Missing argument to %s=.", field); + if (r <= 0) + return parse_log_error(r, field, eq); if (!p) r = sd_bus_message_append(m, "(sv)", "ImportCredential", "as", 1, eq); @@ -1258,7 +1239,7 @@ static int bus_append_standard_input_text(sd_bus_message *m, const char *field, l = cunescape(eq, 0, &unescaped); if (l < 0) - return log_error_errno(l, "Failed to unescape text '%s': %m", eq); + return log_error_errno(l, "Failed to unescape value for %s=: %s", field, eq); if (!strextend(&unescaped, "\n")) return log_oom(); @@ -1276,7 +1257,7 @@ static int bus_append_standard_input_data(sd_bus_message *m, const char *field, r = unbase64mem(eq, &decoded, &sz); if (r < 0) - return log_error_errno(r, "Failed to decode base64 data '%s': %m", eq); + return log_error_errno(r, "Failed to decode base64 data for %s=: %m", field); return bus_append_byte_array(m, field, decoded, sz); } @@ -1290,12 +1271,12 @@ static int bus_try_append_resource_limit(sd_bus_message *m, const char *field, c int rl = rlimit_from_string(suffix); if (rl < 0) - return log_error_errno(rl, "Unknown setting '%s'.", field); + return 0; /* We let the generic error machinery handle this. */ struct rlimit l; r = rlimit_parse(rl, eq, &l); if (r < 0) - return log_error_errno(r, "Failed to parse resource limit: %s", eq); + return parse_log_error(r, field, eq); r = sd_bus_message_append(m, "(sv)", field, "t", (uint64_t) l.rlim_max); if (r < 0) @@ -1343,7 +1324,7 @@ static int bus_append_capabilities(sd_bus_message *m, const char *field, const c r = capability_set_from_string(p, &sum); if (r < 0) - return log_error_errno(r, "Failed to parse %s value %s: %m", field, eq); + return parse_log_error(r, field, eq); sum = invert ? ~sum : sum; @@ -1380,7 +1361,7 @@ static int bus_append_numa_mask(sd_bus_message *m, const char *field, const char } else { r = parse_cpu_set(eq, &nodes); if (r < 0) - return log_error_errno(r, "Failed to parse %s value: %s", field, eq); + return parse_log_error(r, field, eq); } r = cpu_set_to_dbus(&nodes, &array, &allocated); @@ -1428,10 +1409,8 @@ static int bus_append_filter_list(sd_bus_message *m, const char *field, const ch _cleanup_free_ char *word = NULL; r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE); - if (r == -ENOMEM) - return log_oom(); if (r < 0) - return log_error_errno(r, "Invalid syntax: %s", eq); + return parse_log_error(r, field, eq); if (r == 0) break; @@ -1481,7 +1460,7 @@ static int bus_append_namespace_list(sd_bus_message *m, const char *field, const r = namespace_flags_from_string(eq, &flags); if (r < 0) - return log_error_errno(r, "Failed to parse %s value %s.", field, eq); + return parse_log_error(r, field, eq); } if (invert) @@ -1522,7 +1501,7 @@ static int bus_append_bind_paths(sd_bus_message *m, const char *field, const cha r = extract_first_word(&p, &source, ":" WHITESPACE, EXTRACT_UNQUOTE|EXTRACT_DONT_COALESCE_SEPARATORS); if (r < 0) - return log_error_errno(r, "Failed to parse argument: %m"); + return parse_log_error(r, field, eq); if (r == 0) break; @@ -1535,7 +1514,7 @@ static int bus_append_bind_paths(sd_bus_message *m, const char *field, const cha if (p && p[-1] == ':') { r = extract_first_word(&p, &destination, ":" WHITESPACE, EXTRACT_UNQUOTE|EXTRACT_DONT_COALESCE_SEPARATORS); if (r < 0) - return log_error_errno(r, "Failed to parse argument: %m"); + return parse_log_error(r, field, p); if (r == 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Missing argument after ':': %s", eq); @@ -1547,7 +1526,7 @@ static int bus_append_bind_paths(sd_bus_message *m, const char *field, const cha r = extract_first_word(&p, &options, NULL, EXTRACT_UNQUOTE); if (r < 0) - return log_error_errno(r, "Failed to parse argument: %m"); + return parse_log_error(r, field, p); if (isempty(options) || streq(options, "rbind")) flags = MS_REC; @@ -1606,17 +1585,14 @@ static int bus_append_temporary_file_system(sd_bus_message *m, const char *field r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE); if (r < 0) - return log_error_errno(r, "Failed to parse argument: %m"); + return parse_log_error(r, field, eq); if (r == 0) break; w = word; r = extract_first_word(&w, &path, ":", EXTRACT_DONT_COALESCE_SEPARATORS); - if (r < 0) - return log_error_errno(r, "Failed to parse argument: %m"); - if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Failed to parse argument: %s", p); + if (r <= 0) + return parse_log_error(r, field, eq); r = sd_bus_message_append(m, "(ss)", path, w); if (r < 0) @@ -1650,9 +1626,9 @@ static int bus_append_root_hash(sd_bus_message *m, const char *field, const char /* We have a roothash to decode, eg: RootHash=012345789abcdef */ r = unhexmem(eq, &roothash_decoded, &roothash_decoded_size); if (r < 0) - return log_error_errno(r, "Failed to decode RootHash= '%s': %m", eq); + return log_error_errno(r, "Failed to decode base64 data for %s=: %m", field); if (roothash_decoded_size < sizeof(sd_id128_t)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "RootHash= '%s' is too short.", eq); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "%s value '%s' is too short.", field, eq); return bus_append_byte_array(m, field, roothash_decoded, roothash_decoded_size); } @@ -1669,13 +1645,13 @@ static int bus_append_root_hash_signature(sd_bus_message *m, const char *field, if (!(value = startswith(eq, "base64:"))) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Failed to decode %s=%s: neither a path nor starts with 'base64:'.", + "Failed to decode %s value '%s': neither a path nor starts with 'base64:'.", field, eq); /* We have a roothash signature to decode, eg: RootHashSignature=base64:012345789abcdef */ r = unbase64mem(value, &roothash_sig_decoded, &roothash_sig_decoded_size); if (r < 0) - return log_error_errno(r, "Failed to decode %s=%s: %m", field, eq); + return log_error_errno(r, "Failed to decode base64 data for %s=: %m", field); return bus_append_byte_array(m, field, roothash_sig_decoded, roothash_sig_decoded_size); } @@ -1703,7 +1679,7 @@ static int bus_append_root_image_options(sd_bus_message *m, const char *field, c r = strv_split_colon_pairs(&l, p); if (r < 0) - return log_error_errno(r, "Failed to parse argument: %m"); + return parse_log_error(r, field, eq); STRV_FOREACH_PAIR(first, second, l) { r = sd_bus_message_append(m, "(ss)", @@ -1755,14 +1731,14 @@ static int bus_append_mount_images(sd_bus_message *m, const char *field, const c r = extract_first_word(&p, &tuple, NULL, EXTRACT_UNQUOTE|EXTRACT_RETAIN_ESCAPE); if (r < 0) - return log_error_errno(r, "Failed to parse %s property: %s", field, eq); + return parse_log_error(r, field, eq); if (r == 0) break; q = tuple; r = extract_many_words(&q, ":", EXTRACT_CUNESCAPE|EXTRACT_UNESCAPE_SEPARATORS, &first, &second); if (r < 0) - return log_error_errno(r, "Failed to parse %s property: %s", field, eq); + return parse_log_error(r, field, eq); if (r == 0) continue; @@ -1774,7 +1750,7 @@ static int bus_append_mount_images(sd_bus_message *m, const char *field, const c if (isempty(second)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Missing argument after ':' for %s: %s", field, eq); + "Missing argument after ':' for %s=: '%s'", field, eq); r = sd_bus_message_open_container(m, 'r', "ssba(ss)"); if (r < 0) @@ -1793,7 +1769,7 @@ static int bus_append_mount_images(sd_bus_message *m, const char *field, const c r = extract_many_words(&q, ":", EXTRACT_CUNESCAPE|EXTRACT_UNESCAPE_SEPARATORS, &partition, &mount_options); if (r < 0) - return log_error_errno(r, "Failed to parse %s property: %s", field, eq); + return parse_log_error(r, field, eq); if (r == 0) break; /* Single set of options, applying to the root partition/single filesystem */ @@ -1861,14 +1837,14 @@ static int bus_append_extension_images(sd_bus_message *m, const char *field, con r = extract_first_word(&p, &tuple, NULL, EXTRACT_UNQUOTE|EXTRACT_RETAIN_ESCAPE); if (r < 0) - return log_error_errno(r, "Failed to parse %s property: %s", field, eq); + return parse_log_error(r, field, eq); if (r == 0) break; q = tuple; r = extract_first_word(&q, &source, ":", EXTRACT_CUNESCAPE|EXTRACT_UNESCAPE_SEPARATORS); if (r < 0) - return log_error_errno(r, "Failed to parse %s property: %s", field, eq); + return parse_log_error(r, field, eq); if (r == 0) continue; @@ -1895,7 +1871,7 @@ static int bus_append_extension_images(sd_bus_message *m, const char *field, con r = extract_many_words(&q, ":", EXTRACT_CUNESCAPE|EXTRACT_UNESCAPE_SEPARATORS, &partition, &mount_options); if (r < 0) - return log_error_errno(r, "Failed to parse %s property: %s", field, eq); + return parse_log_error(r, field, eq); if (r == 0) break; /* Single set of options, applying to the root partition/single filesystem */ @@ -1950,14 +1926,14 @@ static int bus_append_directory(sd_bus_message *m, const char *field, const char r = extract_first_word(&p, &tuple, NULL, EXTRACT_UNQUOTE); if (r < 0) - return log_error_errno(r, "Failed to parse argument: %m"); + return parse_log_error(r, field, eq); if (r == 0) break; const char *t = tuple; r = extract_many_words(&t, ":", EXTRACT_UNQUOTE|EXTRACT_DONT_COALESCE_SEPARATORS, &source, &dest, &flags); if (r <= 0) - return log_error_errno(r ?: SYNTHETIC_ERRNO(EINVAL), "Failed to parse argument: %m"); + return parse_log_error(r, field, eq); path_simplify(source); @@ -1973,7 +1949,7 @@ static int bus_append_directory(sd_bus_message *m, const char *field, const char } else { ExecDirectoryFlags exec_directory_flags = exec_directory_flags_from_string(flags); if (exec_directory_flags < 0 || (exec_directory_flags & ~_EXEC_DIRECTORY_FLAGS_PUBLIC) != 0) - return log_error_errno(r, "Failed to parse flags: %s", flags); + return log_error_errno(r, "Failed to parse flags for %s=: '%s'", field, flags); if (!isempty(dest)) { path_simplify(dest); @@ -2091,7 +2067,7 @@ static int bus_append_protect_hostname(sd_bus_message *m, const char *field, con const char *colon = strchr(eq, ':'); if (colon) { if (isempty(colon + 1)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to parse argument: %s=%s", field, eq); + return parse_log_error(0, field, eq); _cleanup_free_ char *p = strndup(eq, colon - eq); if (!p) @@ -2121,10 +2097,8 @@ static int bus_append_exit_status(sd_bus_message *m, const char *field, const ch _cleanup_free_ char *word = NULL; r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE); - if (r == -ENOMEM) - return log_oom(); if (r < 0) - return log_error_errno(r, "Invalid syntax in %s: %s", field, eq); + return parse_log_error(r, field, eq); if (r == 0) break; @@ -2148,8 +2122,7 @@ static int bus_append_exit_status(sd_bus_message *m, const char *field, const ch } else /* original r from exit_status_to_string() */ - return log_error_errno(r, "Invalid status or signal %s in %s: %m", - word, field); + return parse_log_error(r, field, word); } r = sd_bus_message_open_container(m, SD_BUS_TYPE_STRUCT, "sv"); @@ -2201,7 +2174,7 @@ static int bus_append_action_exit_status(sd_bus_message *m, const char *field, c r = safe_atou8(eq, &u); if (r < 0) - return log_error_errno(r, "Failed to parse %s=%s", field, eq); + return parse_log_error(r, field, eq); r = sd_bus_message_append(m, "(sv)", field, "i", (int) u); } @@ -2227,7 +2200,7 @@ static int bus_append_timers_monotonic(sd_bus_message *m, const char *field, con usec_t t; r = parse_sec(eq, &t); if (r < 0) - return log_error_errno(r, "Failed to parse %s=%s: %m", field, eq); + return parse_log_error(r, field, eq); r = sd_bus_message_append(m, "(sv)", "TimersMonotonic", "a(st)", 1, field, t); } diff --git a/src/test/test-bus-unit-util.c b/src/test/test-bus-unit-util.c index a5207fa02d..677615a2d3 100644 --- a/src/test/test-bus-unit-util.c +++ b/src/test/test-bus-unit-util.c @@ -142,6 +142,7 @@ TEST(cgroup_properties) { "CPUQuota=200%", "CPUQuotaPeriodSec=100ms", "CPUQuotaPeriodSec=1s", + "-ERANGE CPUQuota=0%", "DeviceAllow=/dev/null rw", "DeviceAllow=/dev/zero r", @@ -382,6 +383,7 @@ TEST(execute_properties) { "CPUSchedulingPriority=50", "CPUSchedulingPriority=1", "CPUSchedulingPriority=99", + "-ERANGE CPUSchedulingPriority=2147483648", "OOMScoreAdjust=100", "OOMScoreAdjust=-100", @@ -579,8 +581,8 @@ TEST(kill_properties) { "KillSignal=1", "KillSignal=64", /* _NSIG == 64 */ - "-EINVAL KillSignal=0", - "-EINVAL KillSignal=65", + "-ERANGE KillSignal=0", + "-ERANGE KillSignal=65", "RestartKillSignal=TERM", "RestartKillSignal=SIGTERM", "FinalKillSignal=WINCH", @@ -591,7 +593,7 @@ TEST(kill_properties) { "ReloadSignal=RTMAX", "ReloadSignal=RTMAX-0", "ReloadSignal=RTMAX-5", - "-EINVAL ReloadSignal=RTMAX-100" + "-ERANGE ReloadSignal=RTMAX-100" ); test_transient_settings_one(UNIT_SCOPE, lines); From 6f06afde598bb2154537f46b32f2dec48123291d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 5 Jul 2025 13:26:07 +0200 Subject: [PATCH 6/9] shared/bus-unit-util: rework error messages for NFTSet= Let's be nice to the user and print the exact reason why we won't accept a setting. --- src/shared/bus-unit-util.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index ebfabcdf36..fde4f239be 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -961,13 +961,21 @@ static int bus_append_nft_set(sd_bus_message *m, const char *field, const char * assert(table); assert(set); - source = nft_set_source_from_string(source_str); + source = r = nft_set_source_from_string(source_str); + if (r < 0) + return log_error_errno(r, "Failed to parse NFT set source '%s': %m", source_str); if (!IN_SET(source, NFT_SET_SOURCE_CGROUP, NFT_SET_SOURCE_USER, NFT_SET_SOURCE_GROUP)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to parse %s.", field); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Bad NFT set source value '%s'.", + nft_set_source_to_string(source)); - nfproto = nfproto_from_string(nfproto_str); - if (nfproto < 0 || !nft_identifier_valid(table) || !nft_identifier_valid(set)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to parse %s.", field); + nfproto = r = nfproto_from_string(nfproto_str); + if (r < 0) + return log_error_errno(r, "Failed to parse nft protocol '%s': %m", nfproto_str); + + if (!nft_identifier_valid(table)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Bad NFT identifier name '%s'.", table); + if (!nft_identifier_valid(set)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Bad NFT identifier name '%s'.", set); r = sd_bus_message_append(m, "(iiss)", source, nfproto, table, set); if (r < 0) From fb98c75e0ea1c38dd8f47833e6dbd766f5d950ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Jul 2025 19:32:51 +0200 Subject: [PATCH 7/9] shared/bus-unit-util: tweak bus_append_exec_command to use Ex prop only if necessary This changes little in behaviour, the conceptual part is more important. The non-Ex variant is the actual name on the command line, and we should use the non-Ex D-Bus property too, if it works. This increases compatibility with old versions. But the code was mostly doing the right thing. Even the tests tested the right thing. Follow-up for b3d593673c5b8b0b7d781fd26ab2062ca6e7dbdb and 898fc00e794d714e2f01409bef440d910c22502a. The test is simplified by taking advantage of the fact that both names on the commandline are supposed to behave identically. Partially resolves https://github.com/systemd/systemd/issues/37174. --- src/shared/bus-unit-util.c | 45 +++++++++---------- .../TEST-23-UNIT-FILE.exec-command-ex.sh | 21 +++------ 2 files changed, 27 insertions(+), 39 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index fde4f239be..48bee05bb3 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -561,9 +561,8 @@ static int bus_append_socket_filter(sd_bus_message *m, const char *field, const static int bus_append_exec_command(sd_bus_message *m, const char *field, const char *eq) { bool explicit_path = false, done = false, ambient_hack = false; _cleanup_strv_free_ char **cmdline = NULL, **ex_opts = NULL; - _cleanup_free_ char *_path = NULL, *upgraded_name = NULL; + _cleanup_free_ char *_path = NULL; ExecCommandFlags flags = 0; - bool is_ex_prop = endswith(field, "Ex"); int r; do { @@ -638,20 +637,18 @@ static int bus_append_exec_command(sd_bus_message *m, const char *field, const c } } while (!done); - if (!is_ex_prop && (flags & (EXEC_COMMAND_NO_ENV_EXPAND|EXEC_COMMAND_FULLY_PRIVILEGED|EXEC_COMMAND_NO_SETUID|EXEC_COMMAND_VIA_SHELL))) { - /* Upgrade the ExecXYZ= property to ExecXYZEx= for convenience */ - is_ex_prop = true; + bool ex_prop = flags & (EXEC_COMMAND_NO_ENV_EXPAND|EXEC_COMMAND_FULLY_PRIVILEGED|EXEC_COMMAND_NO_SETUID|EXEC_COMMAND_VIA_SHELL); + if (ex_prop) { + /* We need to use ExecXYZEx=. */ + if (!endswith(field, "Ex")) + field = strjoina(field, "Ex"); - upgraded_name = strjoin(field, "Ex"); - if (!upgraded_name) - return log_oom(); - field = upgraded_name; - } - - if (is_ex_prop) { r = exec_command_flags_to_strv(flags, &ex_opts); if (r < 0) return log_error_errno(r, "Failed to serialize ExecCommand flags: %m"); + } else { + if (endswith(field, "Ex")) + field = strndupa_safe(field, strlen(field) - 2); } const char *path = NULL; @@ -686,16 +683,16 @@ static int bus_append_exec_command(sd_bus_message *m, const char *field, const c if (r < 0) return bus_log_create_error(r); - r = sd_bus_message_open_container(m, 'v', is_ex_prop ? "a(sasas)" : "a(sasb)"); + r = sd_bus_message_open_container(m, 'v', ex_prop ? "a(sasas)" : "a(sasb)"); if (r < 0) return bus_log_create_error(r); - r = sd_bus_message_open_container(m, 'a', is_ex_prop ? "(sasas)" : "(sasb)"); + r = sd_bus_message_open_container(m, 'a', ex_prop ? "(sasas)" : "(sasb)"); if (r < 0) return bus_log_create_error(r); if (!strv_isempty(cmdline)) { - r = sd_bus_message_open_container(m, 'r', is_ex_prop ? "sasas" : "sasb"); + r = sd_bus_message_open_container(m, 'r', ex_prop ? "sasas" : "sasb"); if (r < 0) return bus_log_create_error(r); @@ -707,8 +704,8 @@ static int bus_append_exec_command(sd_bus_message *m, const char *field, const c if (r < 0) return bus_log_create_error(r); - r = is_ex_prop ? sd_bus_message_append_strv(m, ex_opts) : - sd_bus_message_append(m, "b", FLAGS_SET(flags, EXEC_COMMAND_IGNORE_FAILURE)); + r = ex_prop ? sd_bus_message_append_strv(m, ex_opts) : + sd_bus_message_append(m, "b", FLAGS_SET(flags, EXEC_COMMAND_IGNORE_FAILURE)); if (r < 0) return bus_log_create_error(r); @@ -2584,19 +2581,19 @@ static const BusProperty service_properties[] = { { "FileDescriptorStoreMax", bus_append_safe_atou }, { "RestartSteps", bus_append_safe_atou }, { "ExecCondition", bus_append_exec_command }, + { "ExecConditionEx", bus_append_exec_command }, /* compat */ { "ExecStartPre", bus_append_exec_command }, + { "ExecStartPreEx", bus_append_exec_command }, /* compat */ { "ExecStart", bus_append_exec_command }, + { "ExecStartEx", bus_append_exec_command }, /* compat */ { "ExecStartPost", bus_append_exec_command }, - { "ExecConditionEx", bus_append_exec_command }, - { "ExecStartPreEx", bus_append_exec_command }, - { "ExecStartEx", bus_append_exec_command }, - { "ExecStartPostEx", bus_append_exec_command }, + { "ExecStartPostEx", bus_append_exec_command }, /* compat */ { "ExecReload", bus_append_exec_command }, + { "ExecReloadEx", bus_append_exec_command }, /* compat */ { "ExecStop", bus_append_exec_command }, + { "ExecStopEx", bus_append_exec_command }, /* compat */ { "ExecStopPost", bus_append_exec_command }, - { "ExecReloadEx", bus_append_exec_command }, - { "ExecStopEx", bus_append_exec_command }, - { "ExecStopPostEx", bus_append_exec_command }, + { "ExecStopPostEx", bus_append_exec_command }, /* compat */ { "RestartPreventExitStatus", bus_append_exit_status }, { "RestartForceExitStatus", bus_append_exit_status }, { "SuccessExitStatus", bus_append_exit_status }, diff --git a/test/units/TEST-23-UNIT-FILE.exec-command-ex.sh b/test/units/TEST-23-UNIT-FILE.exec-command-ex.sh index f926e7dea8..f8dceb1dcc 100755 --- a/test/units/TEST-23-UNIT-FILE.exec-command-ex.sh +++ b/test/units/TEST-23-UNIT-FILE.exec-command-ex.sh @@ -20,25 +20,16 @@ property[7_seven]=ExecStopPost # These should all get upgraded to the corresponding Ex property as the non-Ex variant # does not support the ":" prefix (no-env-expand). for c in "${!property[@]}"; do - systemd-run --unit="$c" -r -p "Type=oneshot" -p "${property[$c]}=:/bin/echo \${$c}" /bin/true + systemd-run --unit="$c" -r -p "Type=oneshot" -p "${property[$c]}=:/bin/echo \${$c}" true systemctl show -p "${property[$c]}" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; ignore_errors=no" systemctl show -p "${property[$c]}Ex" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; flags=no-env-expand" done -declare -A property_ex - -property_ex[1_one_ex]=ExecConditionEx -property_ex[2_two_ex]=ExecStartPreEx -property_ex[3_three_ex]=ExecStartEx -property_ex[4_four_ex]=ExecStartPostEx -property_ex[5_five_ex]=ExecReloadEx -property_ex[6_six_ex]=ExecStopEx -property_ex[7_seven_ex]=ExecStopPostEx - -for c in "${!property_ex[@]}"; do - systemd-run --unit="$c" -r -p "Type=oneshot" -p "${property_ex[$c]}=:/bin/echo \${$c}" /bin/true - systemctl show -p "${property_ex[$c]%??}" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; ignore_errors=no" - systemctl show -p "${property_ex[$c]}" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; flags=no-env-expand" +# Ex names on the commandline are supported for backward compat. +for c in "${!property[@]}"; do + systemd-run --unit="${c}_ex" -r -p "Type=oneshot" -p "${property[$c]}Ex=:/bin/echo \${$c}" true + systemctl show -p "${property[$c]}" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; ignore_errors=no" + systemctl show -p "${property[$c]}Ex" "$c" | grep -F "path=/bin/echo ; argv[]=/bin/echo \${$c} ; flags=no-env-expand" done systemd-analyze log-level info From 41288f2daf2c72d0a8614571cfc3de91ecdee372 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 5 Jul 2025 09:22:16 +0200 Subject: [PATCH 8/9] shared/bus-unit-util: fix PrivateTmp=/PrivateUsers=/ProtectControlGroups= and Ex variants For some fields, we perform careful parsing and verification on the sender side. For other fields, we accept any string or strv. I think that actually this is fine: we should optimize for the correct case, i.e. the user runs a command that is valid. The server must perform parsing in all cases, so doing the verification on the sender side doesn't add value. When doing parsing locally, in case of invalid or unsupported input, we would generate the error message locally, so we would avoid the D-Bus call, but the message itself is not better and from the user's point of view, the result is the same. And by doing the parsing only on the server side, we deal better with the case where the sender has an older version of the software. By not doing verification, we implicitly "support" new values. And when the sender has a newer version that supports additional fields, that does not help as long as the server uses an older version. So in case of version mismatches, parsing on the server side is as good or better. Resolves https://github.com/systemd/systemd/issues/37174. --- src/shared/bus-unit-util.c | 38 +++++++++++++++++++++++++++-------- src/test/test-bus-unit-util.c | 14 ++++++++----- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 48bee05bb3..97bf9d264e 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -2088,6 +2088,28 @@ static int bus_append_protect_hostname(sd_bus_message *m, const char *field, con return 1; } +static int bus_append_boolean_or_ex_string(sd_bus_message *m, const char *field, const char *eq) { + int r; + + r = parse_boolean(eq); + if (r >= 0) { + if (endswith(field, "Ex")) + field = strndupa_safe(field, strlen(field) - 2); + + r = sd_bus_message_append(m, "(sv)", field, "b", r); + } else { + if (!endswith(field, "Ex")) + field = strjoina(field, "Ex"); + + /* We allow any string through and let the server perform the verification. */ + r = sd_bus_message_append(m, "(sv)", field, "s", eq); + } + if (r < 0) + return bus_log_create_error(r); + + return 1; +} + static int bus_append_paths(sd_bus_message *m, const char *field, const char *eq) { return bus_append_trivial_array(m, "Paths", eq, "a(ss)", field, eq); @@ -2363,9 +2385,6 @@ static const BusProperty execute_properties[] = { { "SyslogIdentifier", bus_append_string }, { "ProtectSystem", bus_append_string }, { "ProtectHome", bus_append_string }, - { "PrivateTmpEx", bus_append_string }, - { "PrivateUsersEx", bus_append_string }, - { "ProtectControlGroupsEx", bus_append_string }, { "SELinuxContext", bus_append_string }, { "RootImage", bus_append_string }, { "RootVerity", bus_append_string }, @@ -2385,10 +2404,8 @@ static const BusProperty execute_properties[] = { { "TTYVHangup", bus_append_parse_boolean }, { "TTYReset", bus_append_parse_boolean }, { "TTYVTDisallocate", bus_append_parse_boolean }, - { "PrivateTmp", bus_append_parse_boolean }, { "PrivateDevices", bus_append_parse_boolean }, { "PrivateNetwork", bus_append_parse_boolean }, - { "PrivateUsers", bus_append_parse_boolean }, { "PrivateMounts", bus_append_parse_boolean }, { "PrivateIPC", bus_append_parse_boolean }, { "NoNewPrivileges", bus_append_parse_boolean }, @@ -2401,7 +2418,6 @@ static const BusProperty execute_properties[] = { { "ProtectKernelModules", bus_append_parse_boolean }, { "ProtectKernelLogs", bus_append_parse_boolean }, { "ProtectClock", bus_append_parse_boolean }, - { "ProtectControlGroups", bus_append_parse_boolean }, { "MountAPIVFS", bus_append_parse_boolean }, { "BindLogSockets", bus_append_parse_boolean }, { "CPUSchedulingResetOnFork", bus_append_parse_boolean }, @@ -2456,7 +2472,7 @@ static const BusProperty execute_properties[] = { { "LoadCredential", bus_append_load_credential }, { "LoadCredentialEncrypted", bus_append_load_credential }, { "ImportCredential", bus_append_import_credential }, - { "ImportCredentialEx", bus_append_import_credential }, + { "ImportCredentialEx", bus_append_import_credential }, /* compat */ { "LogExtraFields", bus_append_log_extra_fields }, { "LogFilterPatterns", bus_append_log_filter_patterns }, { "StandardInput", bus_append_standard_inputs }, @@ -2491,7 +2507,13 @@ static const BusProperty execute_properties[] = { { "CacheDirectory", bus_append_directory }, { "LogsDirectory", bus_append_directory }, { "ProtectHostname", bus_append_protect_hostname }, - { "ProtectHostnameEx", bus_append_protect_hostname }, + { "ProtectHostnameEx", bus_append_protect_hostname }, /* compat */ + { "PrivateTmp", bus_append_boolean_or_ex_string }, + { "PrivateTmpEx", bus_append_boolean_or_ex_string }, /* compat */ + { "ProtectControlGroups", bus_append_boolean_or_ex_string }, + { "ProtectControlGroupsEx", bus_append_boolean_or_ex_string }, /* compat */ + { "PrivateUsers", bus_append_boolean_or_ex_string }, + { "PrivateUsersEx", bus_append_boolean_or_ex_string }, /* compat */ { NULL, bus_try_append_resource_limit, dump_resource_limits }, {} diff --git a/src/test/test-bus-unit-util.c b/src/test/test-bus-unit-util.c index 677615a2d3..2b21f119e5 100644 --- a/src/test/test-bus-unit-util.c +++ b/src/test/test-bus-unit-util.c @@ -271,11 +271,17 @@ TEST(execute_properties) { "ProtectHome=true", "ProtectHome=read-only", "ProtectHome=tmpfs", - - "PrivateTmpEx=true", - "PrivateTmpEx=false", + "PrivateTmp=true", + "PrivateTmp=fAlSe", + "PrivateTmpEx=0001", + "PrivateTmpEx=0000", + "PrivateTmp=asdf", + "PrivateTmpEx=asdf", + "PrivateUsers=no", + "PrivateUsers=1", "PrivateUsersEx=true", "PrivateUsersEx=false", + "PrivateUsers=whatever", "ProtectControlGroupsEx=true", "ProtectControlGroupsEx=false", @@ -314,10 +320,8 @@ TEST(execute_properties) { "TTYVHangup=true", "TTYReset=true", "TTYVTDisallocate=true", - "PrivateTmp=true", "PrivateDevices=true", "PrivateNetwork=true", - "PrivateUsers=true", "PrivateMounts=true", "PrivateIPC=true", "NoNewPrivileges=true", From 228d84e37ac1e4cbc22f38ff9c78bb1d6bd73690 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 5 Jul 2025 13:01:18 +0200 Subject: [PATCH 9/9] systemd-analyze: stop printing Ex transient settings The test will fail if we ever add one again in the future by mistake. --- src/shared/bus-unit-util.c | 4 +++- test/units/TEST-65-ANALYZE.sh | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 97bf9d264e..41c88b79cf 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -2880,9 +2880,11 @@ void bus_dump_transient_settings(UnitType t) { for (const BusProperty *item = *tables; item->convert; item++) { assert(item->name || item->dump); - /* Do not print deprecated names */ + /* Do not print deprecated names. All "Ex" variants are deprecated. */ if (item->convert == warn_deprecated) continue; + if (item->name && endswith(item->name, "Ex")) + continue; if (item->name) puts(item->name); diff --git a/test/units/TEST-65-ANALYZE.sh b/test/units/TEST-65-ANALYZE.sh index 468c9ed093..61e4a97175 100755 --- a/test/units/TEST-65-ANALYZE.sh +++ b/test/units/TEST-65-ANALYZE.sh @@ -1115,6 +1115,7 @@ systemd-analyze transient-settings mount | grep CPUQuotaPeriodSec (! systemd-analyze transient-settings service | grep CPUAccounting ) (! systemd-analyze transient-settings service | grep ConditionKernelVersion ) (! systemd-analyze transient-settings service | grep AssertKernelVersion ) +(! systemd-analyze transient-settings service socket timer path slice scope mount automount | grep -E 'Ex$' ) systemd-analyze log-level info