Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Jan 4, 2026

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

keszybz and others added 12 commits January 4, 2026 14:07
We have two error messages with exactly the same message.
Let's change one so that it is possible to distinguish them
in logs.
Previously, if execution failed, we'd log at error level both from the
child and the parent, and we were using a bogus variable for the argument
name:
$ build/systemd-inhibit list
Failed to execute : No such file or directory
list failed with exit status 1.

In general, we can and should assume that the program the user is calling
is well behaved, so it'll log the error on its own if appropriate. So we
shouldn't log on "normal errors", but only if the child is terminated by
a signal.

And since the program name is controlled by the user, use quotes everywhere
to avoid ambiguity.

Now:
$ build/systemd-inhibit false
(nothing)
$ build/systemd-inhibit bash -c 'kill -SEGV $$'
src/basic/process-util.c:895: 'bash' terminated by signal SEGV.
The sg_adm and sg_mem fields are supposed to point to a NULL terminated
string array. If these are NULL, some foreign tools like shadow's sg
trigger NULL pointer dereferences (or fortunately their asset() calls).
Fill sg_adm and sg_mem in nss_pack_group_record_shadow to stay
compatible with other NSS getsgnam implementations which set these
members to NULL terminated string arrays.

Tools like shadow's sg would trigger a NULL pointer dereference with
groups only found through nss-systemd otherwise.
Add a test for getsgnam_r to verify that sg_adm and sg_mem always point
to a NULL-terminated string vector.

Extend the gr_mem check of struct group for non-NULL values as well.
In recent Fedora, preset-all fails:
[  155s] Failed to preset unit: File '/buildroot/etc/systemd/user/dbus.service'
         already exists and is a symlink to /usr/lib/systemd/user/dbus-broker.service
[  155s] ‣ "systemctl --root=/buildroot --global preset-all" returned non-zero exit code 1.

Strictly speaking, this is an error in configuration. The presets specify that
both dbus-broker.service and dbus-daemon.service shall be enabled and they both
claim the 'dbus.service' alias. But this kind of error is very easy to make.
Failing the preset operation is too harsh, since in most cases the system will
work fine without an alias and changes in unrelated components can cause the
conflict.

Let's reuse the same logic that was added in
ad5fdd3: when enabling the unit through
'preset' or 'preset-all', print the message, but suppress the error. When
enabling through 'enable', fail the operation.
The `sg_adm` and `sg_mem` fields are not always set in shadow groups,
which can lead to issues with foreign tools like shadow's `sg` command.
Since other NSS implementations properly set these fields and it would
otherwise be impossible to access `administrators` and `members`
information from JSON files, it's bets to always fill these fields.

Even though `sg` is a nice example which should be already installed,
the issue itself can be reproduced with this simple program as well. It
relies on filled `sg_adm` and `sg_mem` fields just like `sg` does:

```
#include <err.h>
#include <gshadow.h>
#include <stdio.h>

int
main(int argc, char *argv[])
{
        struct sgrp *s;
        char **p;

        if (argc != 2)
                errx(1, "usage: poc group");

        s = getsgnam(argv[1]);
        printf("name: %s\n", s->sg_namp);
        printf("admins:\n");
        p = s->sg_adm;
        while (*p != NULL) {
                printf("- %s\n", *p);
                p++;
        }
        printf("members:\n");
        p = s->sg_mem;
        while (*p != NULL) {
                printf("- %s\n", *p);
                p++;
        }
}
```

Run it like this: `./poc root`

Proof of Concept (Arch Linux, which uses systemd with systemd-userdbd
and shadow's sg):

```
$ grep systemd /etc/nsswitch.conf 
passwd: files systemd
group: files [SUCCESS=merge] systemd
shadow: files systemd
gshadow: files systemd
```

Issue with intrinsic groups:

Run as unprivileged user, who has no access to `/etc/gshadow` to trigger
nss-systemd (strace disables setuid of sg)
```
$ strace sg root
write(2, "sg: list.c:169: is_on_list: Asse"..., 61sg: list.c:169: is_on_list: Assertion `NULL != list' failed.
) = 61
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fa7e9c0c000
gettid()                                = 1882
getpid()                                = 1882
tgkill(1882, 1882, SIGABRT)             = 0
--- SIGABRT {si_signo=SIGABRT, si_code=SI_TKILL, si_pid=1882, si_uid=1000} ---
+++ killed by SIGABRT (core dumped) +++
Aborted                    (core dumped) strace sg root
```

Issue with groups through systemd-userdbd:

1. Create a custom group (as root)
```
cat > /etc/userdb/sg-poc.group << EOF
{
  "groupName": "sg-poc",
  "gid": 6123,
  "administrators": [
    "root"
  ],
  "members": [
    "bin"
  ]
}
EOF
ln -s sg-poc.group /etc/userdb/6123.group
```

2. Verify that group actually exists
```
$ userdbctl group sg-poc
  Group name: sg-poc
 Disposition: regular
         GID: 6123
      Admins: root
     Service: io.systemd.NameServiceSwitch
```

3. Run `sg` to switch into group `sg-poc` as regular user, this time
with setuid, i.e. no strace as before
```
$ sg sg-poc
sg: list.c:169: is_on_list: Assertion `NULL != list' failed.
Aborted                    (core dumped) sg sg-poc
```
This has been supported since systemd v242 (specifically commit
7619cb3 if I’m not mistaken; added to NEWS in commit 4107452), but
the man page still claimed otherwise.
…propriate

If these basic sanity checks fail, there's no point in
bumping ratelimit.
@pull pull bot locked and limited conversation to collaborators Jan 4, 2026
@pull pull bot added the ⤵️ pull label Jan 4, 2026
@pull pull bot merged commit 75f04ab into adamlaska:main Jan 4, 2026
1 check failed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants