[Toybox] [PATCH] df: -a should even show mounts we couldn't stat.

[Rob Landley]

On 2/16/21 3:01 AM, Yi-yo Chiang wrote:>     > I think the main purpose of the
original patch is to show mounts whose
>     > stat()/statvfs() failed.
> 
>     And the current one should do that? (I don't have any mount points I can't stat
>     because my laptop doesn't do crazy things with linux security modules, maybe I
>     can put one in a directory I can't read or something to simulate that...)
> 
> 
> Well the cases that I found actually don't stem from Android security models,
> they're actually from the complex bind mounts and over mounts on Android..:( I
> simulated the "weird cases" on my linux workstation by:
> 1. mkdir -p a/b
> 2. mount <tmpfs> on a/b
> 3. mount <tmpfs> on a
> 4. check output of df [-a] # stat a/b would fail, because (2) is over mounted by (3)

Which would only fail if the second tmpfs didn't have a "b" subdirectory. If it
did, you'd presumably get a redundant view of the other filesystem?

> 5. mkdir a/b
> 6. check output of df [-a] # stat a/b would "success", however the st_dev should
> be of (3), not (2)
> 7. umount a
> 8. mount --bind a/b a
> 9. etc etc

Hmmm... with the debian host df I get:

$ sudo mount -t tmpfs uno b/c
$ sudo mount -t tmpfs dos b
$ mkdir b/c
$ df
...
dos               8168980          0   8168980   0% /home/landley/toybox/clean2/b

It edits "uno" out entirely. (Which is what my old one was doing...)

And if I rmdir b/c there's... no change. They're both there in /proc though:

uno /home/landley/toybox/clean2/b/c tmpfs rw,relatime 0 0
dos /home/landley/toybox/clean2/b tmpfs rw,relatime 0 0

Hmmm...

Ok, I taught toybox df -a to show those, _and_ used the existing overmount
cancel logic to hide the false duplicate when a directory for the mount point
exists.

Is that good enough or do you want them to show up without -a?

>     I think it does too, but haven't tested it, which is why I asked.
> 
> 
> Just tested on an Android bench, spotted a few problems:
> 1. show_mt is too aggressively filtering out synthetic filesystems. Would
> "!mt->statvfs.f_blocks && *mt->device != '/'" be a viable heuristic to filter
> out synth fs?

How is / a synthetic filesystem? Shouldn't it be initramfs? I've tested that it
shows initramfs before... yeah, it's still there in mkroot:

  8139cp 0000:00:02.0 eth0: link up, 100Mbps, full-duplex, lpa 0x05E1
  random: fast init done
  Type exit when done.
  # df
  Filesystem     1K-blocks Used Available Use% Mounted on
  rootfs             29892  656     29236   3% /
  dev                29892    0     29892   0% /dev
  #

> 2. df still skipping un-stat()-able mounts.

Try now?

> 3. `df` shows unstatable mounts, however `df <unstatable path/device>` shows
> something such as:
> 
> df: '/mnt/pass_through/0/emulated': Permission denied
> Filesystem     1K-blocks Used Available Use% Mounted on
> df: '/mnt/pass_through/0/emulated': Permission denied

Hmmm...

$ ./df b/c
df: 'b/c': No such file or directory
Filesystem     1K-blocks Used Available Use% Mounted on
df: 'b/c': No such file or directory

Ok, I can at least change that to not give the error message _twice_. But
otherwise, at a design level, that's... sort of right? It _isn't_ there.

There's a problem where if we can't stat it, we can't tell which one it is out
of /proc/mounts because we can't match the device. And you can't do an exact
match on the filename because "df ." is a directory under the mount point. I
would have to add plumbing to xabspath() and then do a longest match
fileunderdir(), which assumes that the /proc/mounts entry is always a cannonical
absolute path? (I _think_ that's true in current kernels but would have to read
the source)...

> this is probably works as intended because the path is unstatable, but we could
> do better by string matching the path/device with mount_points/device in
> /proc/mounts.

We could write a bunch of new code to handle this obscure corner case, yes. But
why? We still wouldn't be able to tell anything _about_ the filesystem we can't
access, and can only indirectly detect its _existence_.

And again, if there _is_ a directory there that's part of an enclosing
filesystem, what should we say about it?

$ df b/c
Filesystem     1K-blocks  Used Available Use% Mounted on
uno              8168980     0   8168980   0% /home/landley/toybox/clean2/b/c
$ ./toybox df b/c
Filesystem     1K-blocks Used Available Use% Mounted on
dos              8168980    0   8168980   0% /home/landley/toybox/clean2/b

The debian one is giving the WRONG ANSWER. (It's dos, not uno.) Mine is giving
what filesystem is actually providing that directory, and when there ISN'T such
a directory, it says so.

Sigh. I suppose I could make it so "df -a dirname" would show all the
overmounts? Except that's always going to include rootfs in the list. Right now
_exact_ overmounts are filtered out (which is why you don't get rootfs when
/dev/sda1 is mounted on / because it's exactly occluded even thought it _is_
technically still there).

This is a design issue. What is the right behavior?

> diff --git a/toys/posix/df.c b/toys/posix/df.c
> index d8ba2986..a04db1d6 100644
> --- a/toys/posix/df.c
> +++ b/toys/posix/df.c
> @@ -93,7 +93,7 @@ static void show_mt(struct mtab_list *mt, int measuring)
>    }
> 
>    // If we don't have -a, skip synthetic filesystems
> -  if (!FLAG(a) && !mt->statvfs.f_blocks) return;
> +  if (!FLAG(a) && !mt->statvfs.f_blocks && *mt->device != '/') return;

This means you won't show network mounts or tmpfs instances. (I.E. this would
break df so my rootfs example above doesn't show rootfs.)

>    // Prepare filesystem display fields
>    *dsuapm = *mt->device == '/' ? xabspath(mt->device, 0) : 0;
> @@ -189,7 +190,7 @@ void df_main(void)
>      // Measure the names then output the table (in filesystem creation order).
>      for (measuring = 1;;) {
>        for (mt = mtstart; mt; mt = mt->next)
> -        if (mt->stat.st_dev) show_mt(mt, measuring);
> +        show_mt(mt, measuring);

I hadn't made it this far down in the email before I already did that. :)

>        if (!measuring--) break;
>        print_header();
>      }
> 
>  
> 
>     > The only question I have left, is it guaranteed that st_dev must be zero
>     or left
>     > unchanged when stat() fails? Or do we need to do something like, "if stat() /
>     > statvfs() fails, ensure st_dev is zero" in portability.c to ensure the caller
>     > knows that stat(mount point) failed?
> 
>     Linux leaves it alone. I can't speak for Apple.
> 
> That's good enough for me, but I can't say for Apple (I don't own an Apple
> station and have never tested on one.)

I had one for a while but never got the apple devkit to work because my apple ID
glitched basically immediately and an apple guru spent an hour trying to unbork
it and couldn't. (I break everything.) Thus I couldn't load xcroak to actually
COMPILE anything on said mac. (It then sat on a shelf for 6 months and I
eventually mailed it to a college student on twitter who needed a new computer...)

Rob