coreutils icon indicating copy to clipboard operation
coreutils copied to clipboard

touch: use special syscall to touch writable files of other users (workaround)

Open BenWiederhake opened this issue 7 months ago • 7 comments

This PR implements a feature that is explicitly tested by the GNU tests (touch/now-owned-by-other).

Sadly, it also reveals how difficult it is to pull off a specific platform-dependent syscall-trick in a way that kinda mostly remains platform-independent.

This PR uses a feature-flag (feat_special_syscall_touch_now, I'm happy to rename it!), instead of trying to determine whether this syscall should be currently working or not.

Some important points to consider:

  • In contrast to #6132, this PR implements the syscall invocation directly. This means we don't get any support if something breaks. It also means that we don't have to wait on upstream.
  • Requiring the user to decide whether the feature can be enabled is ugly and unreliable. I need help from someone who understands how to make this more portable. https://github.com/alexcrichton/filetime/pull/105 does this by implementing fallbacks and in some cases keeping a global atomic boolean to "remember" whether the syscall is supported by the kernel. I think this is unavoidable, because the user might use the same binary on two different Linux kernels, one that supports the "new" utimensat, and one that doesn't. Also, it appears that some platforms need slightly different syscalls, depending on the exact libc implementation. It's a mess.
  • The tests can be run like this:
    $ cargo test test_touch_otheruser_
    $ cargo test --features feat_special_syscall_touch_now test_touch_otheruser_
    
    They assume that we are not running as root, because otherwise there is no need for the feature.

What do you think?

BenWiederhake avatar Apr 13 '25 03:04 BenWiederhake

Changes since first push:

  • make import in test conditional, to appease windows-only testing
  • make definition of constant in test conditional, to appease windows-only testing
  • don't run negative tests as root, since root can do everything anyway

BenWiederhake avatar Apr 13 '25 04:04 BenWiederhake

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

github-actions[bot] avatar Apr 13 '25 05:04 github-actions[bot]

the tooling doesn't detect that it is now passing :/ probably because it runs as root

2025-04-13T04:55:08.8612008Z PASS: tests/touch/now-owned-by-other.sh

sylvestre avatar Apr 13 '25 08:04 sylvestre

actually, it was already passing (because running as root) https://github.com/uutils/coreutils/actions/runs/14427567852/job/40458423529

Upstream runs the test as root: https://github.com/coreutils/coreutils/blob/master/tests/touch/now-owned-by-other.sh#L21

sylvestre avatar Apr 13 '25 08:04 sylvestre

CI failure looks like a flake:'

---- test_tail::test_follow_name_truncate4 stdout ----
bin: "/home/runner.linux/work/target/debug/coreutils"
write(append): /tmp/.tmpzI6YPo/file
run: /home/runner.linux/work/target/debug/coreutils tail -s.1 --max-unchanged-stats=1 -F file
write(truncate): /tmp/.tmpzI6YPo/file

thread 'test_tail::test_follow_name_truncate4' panicked at tests/by-util/test_tail.rs:1993:14:
Expected stderr to be empty, but it's:
tail: file: file truncated

BenWiederhake avatar Apr 13 '25 14:04 BenWiederhake

Upstream runs the test as root: https://github.com/coreutils/coreutils/blob/master/tests/touch/now-owned-by-other.sh#L21

It runs the test setup as root. The actual test invocation runs with non-root:

chroot --skip-chdir --user=$NON_ROOT_USERNAME / env PATH="$PATH" \
  touch -d now root-owned || fail=1

The functionality definitely doesn't exist without this PR, I tried it. I have no explanation why the test was already green, I consider that a separate issue, but have no idea where to begin. In fact, the test should probably be red here, too, because feat_special_syscall_touch_now wasn't set.

Do you have ideas how to deal with the feature flag? I don't want feat_special_syscall_touch_now to end up in production, and you probably neither?

BenWiederhake avatar Apr 13 '25 14:04 BenWiederhake

feat_special_syscall_touch_now => why do you use unix ? and disable capabilities for not supported os in the code ?

i don't like having a new feature (i have been thinking about removing selinux and add management in the code itself)

sylvestre avatar May 04 '25 07:05 sylvestre

no action for a while, closing

sylvestre avatar Oct 06 '25 20:10 sylvestre