<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Feb 17, 2021 at 12:36 AM Rob Landley <<a href="mailto:rob@landley.net" target="_blank" class="cremed">rob@landley.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2/16/21 3:01 AM, Yi-yo Chiang wrote:> > I think the main purpose of the<br>
original patch is to show mounts whose<br>
> > stat()/statvfs() failed.<br>
> <br>
> And the current one should do that? (I don't have any mount points I can't stat<br>
> because my laptop doesn't do crazy things with linux security modules, maybe I<br>
> can put one in a directory I can't read or something to simulate that...)<br>
> <br>
> <br>
> Well the cases that I found actually don't stem from Android security models,<br>
> they're actually from the complex bind mounts and over mounts on Android..:( I<br>
> simulated the "weird cases" on my linux workstation by:<br>
> 1. mkdir -p a/b<br>
> 2. mount <tmpfs> on a/b<br>
> 3. mount <tmpfs> on a<br>
> 4. check output of df [-a] # stat a/b would fail, because (2) is over mounted by (3)<br>
<br>
Which would only fail if the second tmpfs didn't have a "b" subdirectory. If it<br>
did, you'd presumably get a redundant view of the other filesystem?<br>
<br>
> 5. mkdir a/b<br>
> 6. check output of df [-a] # stat a/b would "success", however the st_dev should<br>
> be of (3), not (2)<br>
> 7. umount a<br>
> 8. mount --bind a/b a<br>
> 9. etc etc<br>
<br>
Hmmm... with the debian host df I get:<br>
<br>
$ sudo mount -t tmpfs uno b/c<br>
$ sudo mount -t tmpfs dos b<br>
$ mkdir b/c<br>
$ df<br>
...<br>
dos 8168980 0 8168980 0% /home/landley/toybox/clean2/b<br>
<br>
It edits "uno" out entirely. (Which is what my old one was doing...)<br>
<br>
And if I rmdir b/c there's... no change. They're both there in /proc though:<br>
<br>
uno /home/landley/toybox/clean2/b/c tmpfs rw,relatime 0 0<br>
dos /home/landley/toybox/clean2/b tmpfs rw,relatime 0 0<br>
<br>
Hmmm...<br>
<br>
Ok, I taught toybox df -a to show those, _and_ used the existing overmount<br>
cancel logic to hide the false duplicate when a directory for the mount point<br>
exists.<br>
<br>
Is that good enough or do you want them to show up without -a?<br></blockquote><div><br></div><div>TBH I too am a bit confused with all these testing and comparing output back and forth, and I may have confused some commands and stuff :/</div><div>Anyway I think the behaviour now looks good. "df" hides all un-stat-able and synth mounts, and "df -a" shows everything in /proc/mounts, right?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> I think it does too, but haven't tested it, which is why I asked.<br>
> <br>
> <br>
> Just tested on an Android bench, spotted a few problems:<br>
> 1. show_mt is too aggressively filtering out synthetic filesystems. Would<br>
> "!mt->statvfs.f_blocks && *mt->device != '/'" be a viable heuristic to filter<br>
> out synth fs?<br>
<br>
How is / a synthetic filesystem? Shouldn't it be initramfs? I've tested that it<br>
shows initramfs before... yeah, it's still there in mkroot:<br></blockquote><div><br></div><div>It's a heuristic, I thought devices whose name looks like a path (starts with '/') probably isn't a synthetic FS.</div><div>So, !mt->statvfs.f_blocks && *mt->device != '/'</div><div>=> device has no block and doesn't start with '/', so assume it's synth-fs</div><div>Looking at your "uno&dos" example above, I feel my heuristic to be very fragile. Perhaps a better way is</div><div>!mt->statvfs.f_blocks && !major(mt->stat.st_dev) && mt->stat.st_dev</div><div>=> device has no block, major number is 0 and device is not 0:0 (not a failed stat()), then assume it's synth-fs</div><div>It doesn't matter much anyways since we don't really care why f_blocks is zero (synth-fs or failed stat()), as we are hiding it no matter what.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
8139cp 0000:00:02.0 eth0: link up, 100Mbps, full-duplex, lpa 0x05E1<br>
random: fast init done<br>
Type exit when done.<br>
# df<br>
Filesystem 1K-blocks Used Available Use% Mounted on<br>
rootfs 29892 656 29236 3% /<br>
dev 29892 0 29892 0% /dev<br>
#<br>
<br>
> 2. df still skipping un-stat()-able mounts.<br>
<br>
Try now?<br>
<br>
> 3. `df` shows unstatable mounts, however `df <unstatable path/device>` shows<br>
> something such as:<br>
> <br>
> df: '/mnt/pass_through/0/emulated': Permission denied<br>
> Filesystem 1K-blocks Used Available Use% Mounted on<br>
> df: '/mnt/pass_through/0/emulated': Permission denied<br>
<br>
Hmmm...<br>
<br>
$ ./df b/c<br>
df: 'b/c': No such file or directory<br>
Filesystem 1K-blocks Used Available Use% Mounted on<br>
df: 'b/c': No such file or directory<br>
<br>
Ok, I can at least change that to not give the error message _twice_. But<br>
otherwise, at a design level, that's... sort of right? It _isn't_ there.<br>
<br>
There's a problem where if we can't stat it, we can't tell which one it is out<br>
of /proc/mounts because we can't match the device. And you can't do an exact<br>
match on the filename because "df ." is a directory under the mount point. I<br>
would have to add plumbing to xabspath() and then do a longest match<br>
fileunderdir(), which assumes that the /proc/mounts entry is always a cannonical<br>
absolute path? (I _think_ that's true in current kernels but would have to read<br>
the source)...<br>
<br>
> this is probably works as intended because the path is unstatable, but we could<br>
> do better by string matching the path/device with mount_points/device in<br>
> /proc/mounts.<br>
<br>
We could write a bunch of new code to handle this obscure corner case, yes. But<br>
why? We still wouldn't be able to tell anything _about_ the filesystem we can't<br>
access, and can only indirectly detect its _existence_.<br>
<br></blockquote><div><br></div><div>I was thinking of a case, where stat(/dev/block/blah) failed but there is a mounted entry of /dev/block/blah in /proc/mounts, then we should still show the /proc/mounts line because the device name is an exact match.</div><div>But yes I agree with you now, this is still a flawed case because /dev & /dev/block/ might be overmounted... We can never be sure what failed to stat(/dev/block/blah) actually means, it could mean the user don't have enough permission, or /dev/block/blah is hidden to the user etc etc. I think having "df -a" to show inaccessible / shadowed mounts is good enough. </div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
And again, if there _is_ a directory there that's part of an enclosing<br>
filesystem, what should we say about it?<br>
<br>
$ df b/c<br>
Filesystem 1K-blocks Used Available Use% Mounted on<br>
uno 8168980 0 8168980 0% /home/landley/toybox/clean2/b/c<br>
$ ./toybox df b/c<br>
Filesystem 1K-blocks Used Available Use% Mounted on<br>
dos 8168980 0 8168980 0% /home/landley/toybox/clean2/b<br>
<br>
The debian one is giving the WRONG ANSWER. (It's dos, not uno.) Mine is giving<br>
what filesystem is actually providing that directory, and when there ISN'T such<br>
a directory, it says so.<br>
<br>
Sigh. I suppose I could make it so "df -a dirname" would show all the<br>
overmounts? Except that's always going to include rootfs in the list. Right now<br>
_exact_ overmounts are filtered out (which is why you don't get rootfs when<br>
/dev/sda1 is mounted on / because it's exactly occluded even thought it _is_<br>
technically still there).<br>
<br></blockquote><div><br></div><div>Sorry but I don't understand, I thought exact overmounts are shown already?</div><div>$ sudo mount a.img a/b</div><div>$ sudo mount b.img a/b</div><div>$ ./df -a</div><div>/dev/loop0 - - - - /ssd2/external/toybox/a/b<br>/dev/loop1 2846 45 2521 2% /ssd2/external/toybox/a/b<br></div><div><br></div><div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
This is a design issue. What is the right behavior?<br>
<br>
> diff --git a/toys/posix/df.c b/toys/posix/df.c<br>
> index d8ba2986..a04db1d6 100644<br>
> --- a/toys/posix/df.c<br>
> +++ b/toys/posix/df.c<br>
> @@ -93,7 +93,7 @@ static void show_mt(struct mtab_list *mt, int measuring)<br>
> }<br>
> <br>
> // If we don't have -a, skip synthetic filesystems<br>
> - if (!FLAG(a) && !mt->statvfs.f_blocks) return;<br>
> + if (!FLAG(a) && !mt->statvfs.f_blocks && *mt->device != '/') return;<br>
<br>
This means you won't show network mounts or tmpfs instances. (I.E. this would<br>
break df so my rootfs example above doesn't show rootfs.)<br>
<br>
> // Prepare filesystem display fields<br>
> *dsuapm = *mt->device == '/' ? xabspath(mt->device, 0) : 0;<br>
> @@ -189,7 +190,7 @@ void df_main(void)<br>
> // Measure the names then output the table (in filesystem creation order).<br>
> for (measuring = 1;;) {<br>
> for (mt = mtstart; mt; mt = mt->next)<br>
> - if (mt->stat.st_dev) show_mt(mt, measuring);<br>
> + show_mt(mt, measuring);<br>
<br>
I hadn't made it this far down in the email before I already did that. :)<br>
<br>
> if (!measuring--) break;<br>
> print_header();<br>
> }<br>
> <br>
> <br>
> <br>
> > The only question I have left, is it guaranteed that st_dev must be zero<br>
> or left<br>
> > unchanged when stat() fails? Or do we need to do something like, "if stat() /<br>
> > statvfs() fails, ensure st_dev is zero" in portability.c to ensure the caller<br>
> > knows that stat(mount point) failed?<br>
> <br>
> Linux leaves it alone. I can't speak for Apple.<br>
> <br>
> That's good enough for me, but I can't say for Apple (I don't own an Apple<br>
> station and have never tested on one.)<br>
<br>
I had one for a while but never got the apple devkit to work because my apple ID<br>
glitched basically immediately and an apple guru spent an hour trying to unbork<br>
it and couldn't. (I break everything.) Thus I couldn't load xcroak to actually<br>
COMPILE anything on said mac. (It then sat on a shelf for 6 months and I<br>
eventually mailed it to a college student on twitter who needed a new computer...)<br>
<br>
Rob<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><table width="90%" border="0" cellspacing="0" cellpadding="0" style="margin:0px;padding:0px;font-family:"Times New Roman";max-width:348px"><tbody style="margin:0px;padding:0px"><tr style="margin:0px;padding:0px"><td style="padding:0px"><table border="0" cellspacing="0" cellpadding="0" style="margin:0px;padding:20px 0px 0px"><tbody style="margin:0px;padding:0px"><tr style="margin:0px;padding:0px"><td valign="top" style="padding:0px 20px 0px 0px;vertical-align:top;border-right:1px solid rgb(213,213,213)"><img src="https://i.imgur.com/eGpkLls.png" width="200" height="64"><br></td><td style="padding:0px 0px 0px 20px"><table border="0" cellspacing="0" cellpadding="0" style="margin:0px;padding:0px"><tbody style="margin:0px;padding:0px"><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:1px 0px 5px;font-size:13px;line-height:13px;color:rgb(56,58,53);font-weight:700">Yi-yo Chiang</td></tr><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:0px 0px 5px;font-size:11px;line-height:13px;color:rgb(56,58,53)">Software Engineer</td></tr><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:0px 0px 5px;font-size:11px;line-height:13px;color:rgb(56,58,53)"><a href="mailto:yochiang@google.com" target="_blank" class="cremed">yochiang@google.com</a></td></tr><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:0px 0px 3px;font-size:11px;line-height:13px;color:rgb(3,112,248)"></td></tr></tbody></table></td></tr></tbody></table></td></tr></tbody></table></div></div></div>