On Mon, Feb 15, 2021 at 10:31 PM Rob Landley <r...@landley.net> wrote:
> Sigh, thunderbird crashed and I lost 8 gazillion open reply windows with > half-composed messages. (Kmail used to remember messages being composed > and open > them back up again when it was relaunched. I miss kmail, but they tied it > to a > boat anchor of a desktop...) > > Oh well, makes closing down my laptop so I can do a distro version upgrade > only > a half day instead of a full day's work... > > On 2/14/21 1:32 PM, Yi-yo Chiang wrote: > > Hi Rob, > > > > 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) 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 > > For example, an under-privileged user may be able to read /proc/mounts > but lack > > the permission to stat(vfs) the mount point, so showing "-" is a way of > saying > > "I know this XXX device is being mount on YYY mount point, however for > whatever > > reason I lack the means to get usage information from the filesystem". > > And the test I'm using is that the dev node is zero, which should never > happen > because you can't mount a NULL device. Instead of adding an external flag, > it's > the same in-band signaling it was using before. > > Previously, the logic would skip such mounts (you could get them in the > list of > mount points if all you wanted was the path, but we had no info about them > so df > would skip them). Elliott apparently wanted to display them as - - - - > entries > and I _think_ the current code does that but haven't tested it because I > can > stat all my filesystems because I'm not crazy enough to use LFS. > > > So I think 5f5f97f215bb accomplishes what the original change wants by > not > > skipping 0:0 device at all and gives "st_dev == 0" a special meaning of > > "stat(mount point) failed". > > 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? 2. df still skipping un-stat()-able mounts. 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 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. 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; // 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); 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.) > Rob > Thanks! -- Yi-yo Chiang Software Engineer yochi...@google.com
_______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net