<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Feb 15, 2021 at 10:31 PM Rob Landley <<a href="mailto:rob@landley.net">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">Sigh, thunderbird crashed and I lost 8 gazillion open reply windows with<br>
half-composed messages. (Kmail used to remember messages being composed and open<br>
them back up again when it was relaunched. I miss kmail, but they tied it to a<br>
boat anchor of a desktop...)<br>
<br>
Oh well, makes closing down my laptop so I can do a distro version upgrade only<br>
a half day instead of a full day's work...<br>
<br>
On 2/14/21 1:32 PM, Yi-yo Chiang wrote:<br>
> Hi Rob,<br>
> <br>
> I think the main purpose of the 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></blockquote><div><br></div><div>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:</div><div>1. mkdir -p a/b</div><div>2. mount <tmpfs> on a/b</div><div>3. mount <tmpfs> on a</div><div>4. check output of df [-a] # stat a/b would fail, because (2) is over mounted by (3)</div><div>5. mkdir a/b</div><div>6. check output of df [-a] # stat a/b would "success", however the st_dev should be of (3), not (2)</div><div>7. umount a</div><div>8. mount --bind a/b a</div><div>9. etc etc</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> For example, an under-privileged user may be able to read /proc/mounts but lack<br>
> the permission to stat(vfs) the mount point, so showing "-" is a way of saying<br>
> "I know this XXX device is being mount on YYY mount point, however for whatever<br>
> reason I lack the means to get usage information from the filesystem".<br>
<br>
And the test I'm using is that the dev node is zero, which should never happen<br>
because you can't mount a NULL device. Instead of adding an external flag, it's<br>
the same in-band signaling it was using before.<br>
<br>
Previously, the logic would skip such mounts (you could get them in the list of<br>
mount points if all you wanted was the path, but we had no info about them so df<br>
would skip them). Elliott apparently wanted to display them as - - - - entries<br>
and I _think_ the current code does that but haven't tested it because I can<br>
stat all my filesystems because I'm not crazy enough to use LFS.<br>
<br>
> So I think 5f5f97f215bb accomplishes what the original change wants by not<br>
> skipping 0:0 device at all and gives "st_dev == 0" a special meaning of<br>
> "stat(mount point) failed".<br>
<br>
I think it does too, but haven't tested it, which is why I asked.<br>
<br></blockquote><div><br></div><div>Just tested on an Android bench, spotted a few problems:</div><div>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?</div><div>2. df still skipping un-stat()-able mounts.</div><div>3. `df` shows unstatable mounts, however `df <unstatable path/device>` shows something such as:</div><div><br></div><div>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></div><div><br></div><div>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.</div><div><br></div><div>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> // 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> if (!measuring--) break;<br> print_header();<br> }<br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> The only question I have left, is it guaranteed that st_dev must be zero 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></blockquote><div><br></div><div>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.)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Rob<br>
</blockquote></div><br clear="all"><div>Thanks!</div>-- <br><div dir="ltr" class="gmail_signature"><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>