libsemanage: use mv instead of rename for container compat

[jmarrero]

Initial good path for: https://github.com/SELinuxProject/selinux/issues/343 but I am sure it needs input validation and/or error handling.

[cgzones]

This should probably implemented in C instead of spawning a new shell.

Untested draft:

static int write_full(int fd, const void *data, size_t count)
{
        const unsigned char *p = (const unsigned char *)data;

        while (count > 0) {
                ssize_t n_rw;
                do {
                        n_rw = write(fd, p, count);
                } while (n_rw < 0 && errno == EINTR);
                if (n_rw < 0)
                        return -1;
                if (n_rw == 0)
                        break;
                p += n_rw;
                count -= (size_t)n_rw;
        }

        return 0;
}

static int mv(const char *src, const char *dest)
{
        struct stat oldst;
        char buffer[8192];
        int old = -1, new = -1, r;

        r = rename(src, dest);
        if (r == 0)
                return 0;
        if (errno != EXDEV)
                return -1;

        old = open(src, O_RDONLY | O_CLOEXEC);
        if (old < 0)
                return -1;

        r = fstat(old, &oldst);
        if (r < 0)
                goto err;

        r = unlink(dest);
        if (r < 0 && errno != ENOENT)
                goto err;

        new = open(dest, O_WRONLY | O_CLOEXEC | O_CREAT | O_EXCL, ACCESSPERMS & oldst.st_mode);
        if (new < 0)
                goto err;

        for (;;) {
                ssize_t n;

                n = read(old, buffer, sizeof(buffer));
                if (n == 0)
                        break;
                if (n > 0) {
                        r = write_full(new, buffer, (size_t)n);
                        if (r < 0)
                                goto err;
                        continue;
                }

                if (errno == EINTR)
                        continue;

                goto err;
        }

        close(old);
        r = close(new);
        if (r < 0)
                return -1;

        r = unlink(src);
        if (r < 0)
                return -1;

        return 0;
err:
        if (new >= 0)
                close(new);
        if (old >= 0)
                close(old);
        return -1;
}

[jmarrero]

Thanks @cgzones I will give this a try and report back.

[bachradsusi]

Before you send it to the review on selinux@vger.kernel.org could you please add provide more information in the commit message about the problem this tries to fix?

Especially about the filesystem layout and why /etc/selinux/targeted/active is on a different filesystem than /etc/selinux/targeted/previous. What would be the impact on the filesystem layout when /etc/selinux/targeted/active is copied into /etc/selinux/targeted/previous and /etc/selinux/targeted/tmp, unlink()'ed and then created again with a copy of /etc/selinux/targeted/tmp?

[WOnder93]

@bachradsusi They are not on different filesystems - the problem is that overlayfs doesn't support rename(2) even on the same mount in some circumstances: https://docs.docker.com/storage/storagedriver/overlayfs-driver/#modifying-files-or-directories

Anyway, I don't think @cgzones's solution will work in this case, since we are renaming a directory, not a single file. Also, I believe we expect the rename here to be atomic, so we need to ensure that whatever fallback we use for EXDEV is still atomic (at least to the same extent as rename(2) is).

[bachradsusi]

thanks @WOnder93 this was the information I was missing.

Btw libsemanage already implements copying directories see https://github.com/SELinuxProject/selinux/blob/master/libsemanage/src/semanage_store.c#L773

[jmarrero]

Sorry for the week delay, went on PTO just after my update here.

@cgzones using your draft I get:

libsemanage.semanage_commit_sandbox: Error while renaming /etc/selinux/targeted/active to /etc/selinux/targeted/previous. (Is a directory).

Which is what @WOnder93 suggested.

As for using @bachradsusi suggestion, I get:

Installing: usbguard-selinux-1.1.0-1.fc35.noarch (updates)
libsemanage.semanage_commit_sandbox: Error while renaming /etc/selinux/targeted/tmp to /etc/selinux/targeted/active. (File exists).
libsemanage.semanage_commit_sandbox: Error while renaming /etc/selinux/targeted/previous back to /etc/selinux/targeted/active. (File exists).
/usr/sbin/semodule:  Failed!

Also I tried a combination of both suggestions by replacing the rename on: https://github.com/SELinuxProject/selinux/blob/master/libsemanage/src/semanage_store.c#L763 with cgzones's mv implementation and using semanage_copy_dir for the other rename calls. But ended in the same error as above.

[bachradsusi]

@jmarrero Please try the https://github.com/bachradsusi/SELinuxProject-selinux/commit/03db46475a4c47a3e8bc875f2f901df3649371fd

It implements fallback to semanage_copy_dir() and semanage_remove_dir() if rename() failed on EXDEV.

Even though it's not an atomic operation, it should happen just once (based on my observation).

If it works for you, we should move this to selinux@vger.kernel.org

[jmarrero]

It works on my end. Thank you!

[bachradsusi]

https://github.com/SELinuxProject/selinux/commit/c7a3b93e31df312ed5b71436ec874054a95d4209

[cgwalters]

cc https://github.com/coreos/coreos-layering-examples/pull/16