[systemd-devel] Incorrect logic when /etc/machine-id is missing

[Jan Synacek]

Hello,

I believe that the following code is not correct:

src/core/machine-id-setup.c:188-190:

mkdir_parents(etc_machine_id, 0755);
fd = open(etc_machine_id, O_RDWR|O_CREAT|O_CLOEXEC|O_NOCTTY, 0444);
if (fd >= 0)
        writable = true;
else {
        fd = open(etc_machine_id, O_RDONLY|O_CLOEXEC|O_NOCTTY);
        if (fd < 0) {
                log_error("Cannot open %s: %m", etc_machine_id);
                return -errno;
        }

        writable = false;
}

src/core/machine-id-setup.c:218-249:

r = generate(id, root);
if (r < 0)
        return r;

if (S_ISREG(st.st_mode) && writable) {
        lseek(fd, 0, SEEK_SET);

        if (loop_write(fd, id, 33, false) == 33)
                return 0;
}

fd = safe_close(fd);

/* Hmm, we couldn't write it? So let's write it to
 * /run/machine-id as a replacement */

RUN_WITH_UMASK(0022) {
        r = write_string_file(run_machine_id, id);
}
if (r < 0) {
        log_error("Cannot write %s: %s", run_machine_id, strerror(-r));
        unlink(run_machine_id);
        return r;
}

/* And now, let's mount it over */
r = mount(run_machine_id, etc_machine_id, NULL, MS_BIND, NULL);
if (r < 0) {
        log_error("Failed to mount %s: %m", etc_machine_id);
        unlink_noerrno(run_machine_id);
        return -errno;
}

If /etc/machine-id is missing on the system, the first open() call
should probably handle that case. That's actually not true (at least on
my system), because the underlying filesystem is read-only at that
time. The second open() call fails as well, because there's no
/etc/machine-id, resulting in a boot failure.

I tried briding the code so that if the second open() fails, it jumps to
the line 218. It then correctly generates the id in /run/machine-id,
however the mount() call fails, because it's not possible to bind
mount a file to nowhere, i.e. the destination always has to exist. The
boot fails as well.

Now I was determined to fix this bug, however I'm left clueless as to
how this is actually supposed to work. Is the entire logic in this piece
of code wrong, or am I missing something? How is the
(re)generating/mounting of machine-id supposed to work?

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat