Page MenuHomeFreeBSD

Don't leave `path` behind when executing `:chflags_success`
ClosedPublic

Authored by ngie on Jul 4 2020, 4:59 PM.

Details

Summary

Prior to this change a SF_IMMUTABLE chflagsat(2)'ed file (path) was left
behind, which sabotaged kyua(1) from being able to clean up the work directory,
This resulted in unnecessary work for folks having to clean up the work
directory on non-disposable systems, which defaults to /tmp. Use UF_OFFLINE
instead of SF_IMMUTABLE, in part because setting SF_IMMUTABLE isn't relevant
to the test and SF_IMMUTABLE cannot be cleared at all securelevels, as pointed
out by @asomers.

Additional work is required to catch cases like this upfront in the future to
avoid tester headache. See https://github.com/jmmv/kyua/issues/142 for more details.

Suggested by: asomers
MFC after: 1 week
PR: 247761
Sponsored by: DellEMC

Test Plan
$ sudo sh -c 'chflags -R 0 /tmp/kyua.*; rm -Rf /tmp/kyua.*'
chflags: /tmp/kyua.*: No such file or directory
$ sudo kyua test -k /usr/tests/sys/audit/Kyuafile file-attribute-modify
...

Results file id is usr_tests_sys_audit.20200711-145256-203233
Results saved to /root/.kyua/store/results.usr_tests_sys_audit.20200711-145256-203233.db

53/53 passed (0 failed)
$ ls /tmp/kyua*
ls: /tmp/kyua*: No such file or directory
$

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ngie requested review of this revision.Jul 4 2020, 4:59 PM
asomers requested changes to this revision.Jul 4 2020, 6:47 PM
asomers added a subscriber: aniketp.

Instead of clearing flags, which might not be possible in a higher securelevel, how about just using UF_OFFLINE instead of SF_IMMUTABLE? That's what some of the other test cases do. The test doesn't care what the flag actually is; it just needs to be something.

This revision now requires changes to proceed.Jul 4 2020, 6:47 PM

Instead of clearing flags, which might not be possible in a higher securelevel, how about just using UF_OFFLINE instead of SF_IMMUTABLE? That's what some of the other test cases do. The test doesn't care what the flag actually is; it just needs to be something.

Ack. That reasoning makes a whole lot of sense :).

ngie edited the test plan for this revision. (Show Details)

Use UF_OFFLINE instead of SF_IMMUTABLE, as recommended by @asomers.

This revision is now accepted and ready to land.Jul 11 2020, 3:13 PM
tests/sys/audit/file-attribute-modify.c
714 ↗(On Diff #74303)

I'll remove this spurious whitespace change in the commit.