[Toybox] ltp test failure with toybox mv

[Rob Landley]

On 7/7/21 7:36 PM, enh via Toybox wrote:
> here's a weird one...
> 
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh#L131
> <https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh#L131>
>
> test6 fails with toybox mv because toybox (reasonably enough) believes that if
> it failed to unlink the _files_ it obviously can't unlink their parent
> directory. but apparently that's not true for Linux cgroups... there the file
> unlinks fail but the directory unlink succeeds:

bra fscking vo.

Did you know bash has a &> syntax to do the whole > target 2>&1 dance in a
single redirect? (I only know because I had to implement it for toysh, but it's
quite useful...)

Sigh, what are $start_path and $PWD here? I'm assuming this test is doing a mv
on a directory, not on a file? And the directory is refusing rename() so it's
having to do components?

> openat(AT_FDCWD, "/dev/cgroup/ltp/ltp_1", O_RDONLY|O_CLOEXEC) = 4
> ...
> unlinkat(4, "cgroup.procs", 0)          = -1 EPERM (Operation not permitted)
> unlinkat(4, "tasks", 0)                 = -1 EPERM (Operation not permitted)
> ...
> unlinkat(AT_FDCWD, "/dev/cgroup/ltp/ltp_1", AT_REMOVEDIR) = 0

Lovely. (Is this running as root, or as a normal user?)

> coreutils on the other hand seems from strace to not even try to remove the
> parent directory if there was an earlier failure.

Ok, the failure is removing a directory we shouldn't?

> it seems like toybox's decision about whether to unlink the source...
> 
>   // Did we make a thing?
>   if (fdout != -1) {
>     ...
>     if (CFG_MV && toys.which->name[0] == 'm' && !err)
>       if (unlinkat(tfd, try->name, S_ISDIR(try->st.st_mode) ? AT_REMOVEDIR :0))
>         err = "%s";
> 
> ...really is what it says in the comment: did we _create_ something? not "did we
> remove everything?", because why would you need to ask that? if there's one
> thing we all know about rmdir(2) it's that the directory must be empty.
> 
> expect, apparently, for cgroups, where that isn't true.

Sigh. Ok. A filesystem has broken magic behavior that needs to be worked around.
Not the first. (Reiserfs used to be full of "unique" behaviors, and once upon a
time people cared. Given that cgroups2 was written for the systemd people to
replace the more generic cgroups1, I'm not at all surprised it's crap.)

> i scratched my head for a while about how to get that behavior in toybox
> (assuming the resolution is to sigh, shake our fists at the clouds, and work
> around this linux oddity), and it seems like we could use the fact that
> error_msg() will have incremented toys.exitval, and add that to the CFG_MV `if`
> above?

Sibling directories. If we're not stopping at one error then we may pop up one
and descend down again, and THOSE we still want to delete. It's gotta either
pass through the return code or the child needs to poke ->parent.

Sigh, I'll throw it on the todo heap. How time critical is this?

Rob

P.S. The logical thing to do is return DIRTREE_SAVE for the failure to delete
and then have the parent check to see if it ->child is null...