New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tmpfiles dont resolve pathnames when traversing recursively #8358

Merged
merged 5 commits into from Mar 5, 2018

Conversation

3 participants
@fbuihuu
Copy link
Contributor

fbuihuu commented Mar 5, 2018

No description provided.

@keszybz
Copy link
Member

keszybz left a comment

The tests are failing.

First of all, the tests fail on repeated installation:

sudo make -C test/TEST-22-TMPFILES BUILD_DIR=../../build setup run; sudo make -C test/TEST-22-TMPFILES BUILD_DIR=../../build setup run

The following should (at least partially) fix that:

─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
renamed: a/test/TEST-22-TMPFILES/test.sh to b/test/TEST-22-TMPFILES/test.sh
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ test.sh:19 @ test_setup() {
    inst_binary xargs

    # mask some services that we do not want to run in these tests
    ln -s /dev/null $initdir/etc/systemd/system/systemd-hwdb-update.service
    ln -s /dev/null $initdir/etc/systemd/system/systemd-journal-catalog-update.service
    ln -s /dev/null $initdir/etc/systemd/system/systemd-networkd.service
    ln -s /dev/null $initdir/etc/systemd/system/systemd-networkd.socket
    ln -s /dev/null $initdir/etc/systemd/system/systemd-resolved.service
    ln -s /dev/null $initdir/etc/systemd/system/systemd-machined.service
    ln -fs /dev/null $initdir/etc/systemd/system/systemd-hwdb-update.service
    ln -fs /dev/null $initdir/etc/systemd/system/systemd-journal-catalog-update.service
    ln -fs /dev/null $initdir/etc/systemd/system/systemd-networkd.service
    ln -fs /dev/null $initdir/etc/systemd/system/systemd-networkd.socket
    ln -fs /dev/null $initdir/etc/systemd/system/systemd-resolved.service
    ln -fs /dev/null $initdir/etc/systemd/system/systemd-machined.service

    # setup the testsuite service
    cp testsuite.service $initdir/etc/systemd/system/
    setup_testsuite

    mkdir $initdir/testsuite
    mkdir -p $initdir/testsuite
    cp run-tmpfiles-tests.sh $initdir/testsuite/
    cp test-*.sh $initdir/testsuite/

But it still fails with:

Mar 05 18:28:05 systemd-testsuite systemd-tmpfiles[22]: Assertion 'path' failed at ../src/basic/selinux-util.c:129, function mac_selinux_fix(). Aborting.
unsigned f;
int r;
int r;

This comment has been minimized.

@keszybz

keszybz Mar 5, 2018

Member

Something is off with indentation here. Starting from this line until line 1209, indentation is 9 spaces, not 8.

fbuihuu added some commits Mar 2, 2018

tmpfiles: don't resolve pathnames when traversing recursively through…
… directory trees

Otherwise we can be fooled if one path component is replaced underneath us.

The patch achieves that by always operating at file descriptor level (by using
*at() helpers) and by making sure we do not any path resolution when traversing
direcotry trees.

However this is not always possible, for instance when listing the content of a
directory or some operations don't provide the *at() helpers or others (such as
fchmodat()) don't have the AT_EMPTY_PATH flag. In such cases we operate on
/proc/self/fd/%i pseudo-symlink instead, which works the same for all kinds of
objects and requires no checking of type beforehand.

Also O_PATH flag is used when opening file objects in order to prevent
undesired behaviors: device nodes from reacting, automounts from
triggering, etc...

Fixes: #7986
Fixes: CVE-2018-6954

@fbuihuu fbuihuu force-pushed the fbuihuu:tmpfiles-dont-resolve-pathnames-when-traversing-recursively branch from 0df5997 to ed9e508 Mar 5, 2018

@fbuihuu

This comment has been minimized.

Copy link
Contributor

fbuihuu commented Mar 5, 2018

@keszybz thanks for your feedback. I fixed the testsuite so it can work without needing to clean it up every time.

Regarding the assertion, it's also fixed. I didn't notice it because SELINUX support is disabled here.

@keszybz keszybz force-pushed the fbuihuu:tmpfiles-dont-resolve-pathnames-when-traversing-recursively branch from ed9e508 to e04fc13 Mar 5, 2018

@keszybz

This comment has been minimized.

Copy link
Member

keszybz commented Mar 5, 2018

@fbuihuu test-02.sh consistently fails for me. I force-pushed into your branch to remove it for now, because I want to merge this. I think test-02.sh is just to brittle to merge (I played around with MAX_BUSY_FILES and stuff, but I couldn't get it to work). I think it'll need to be done differently somehow.

@keszybz keszybz merged commit e6131c6 into systemd:master Mar 5, 2018

2 of 5 checks passed

Fedora Rawhide CI x86_64 rpm build [failed]
Details
artful-i386 autopkgtest running
Details
xenial-amd64 autopkgtest running
Details
artful-s390x autopkgtest finished (success)
Details
semaphoreci The build passed on Semaphore.
Details
@fbuihuu

This comment has been minimized.

Copy link
Contributor

fbuihuu commented Mar 6, 2018

@keszybz wow it was fast.... maybe too fast...

I wished we could have investigated why the test is failing on your side (what was the error BTW) at least ?

And this change deserves more testing/reviews IMHO, not sure it's v238 material.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment