[Toybox] [PATCH] dirtree.c: avoid spurious EINVAL warnings.

[enh]

On Sat, Dec 14, 2019 at 3:37 PM Rob Landley <rob at landley.net> wrote:
>
> On 12/13/19 6:36 PM, enh via Toybox wrote:
> > An Android engineer complained that they were seeing this when not
> > running as root:
> >
> >   $ adb shell ls
> >   ls: ./postinstall: Invalid argument
> >   ls: ./init: Permission denied
> >   ls: ./data_mirror: Invalid argument
> >   ls: ./init.environ.rc: Invalid argument
> >   ls: ./metadata: Invalid argument
> >   acct
> >   adb_keys
> >   apex
>
> Are you sure that's current? init isn't listed to stdout: the current code is
> saying "permission denied" but also listing them.

yes, this was ToT. note that by default adb shell doesn't allocate a
pty, so you're seeing all the stderr before you see any of the stdout.

`adb shell -t` gets you a pty *if* you have a tty yourself. `adb shell
-tt` forces a pty. this was copied from ssh, but in retrospect i think
we should have defaulted to the `-t` behavior. i assume it would be
too disruptive to change again now.

> I think the goto error would discard postinstall and such, yes. (Still don't
> have this selinux test environment set up...)
>
> >>From strace, it was here:
> >
> >   newfstatat(4, "adb_keys", 0x7fc67eca88, AT_SYMLINK_NOFOLLOW) = -1
> > EACCES (Permission denied)
> >   readlinkat(4, "adb_keys", 0x5e843c7720, 4095) = -1 EINVAL (Invalid argument)
> >
> > So stop looking at st.st_mode (and then deciding to do a readlinkat())
> > if we didn't actually successfully stat().
>
> I'm wondering if I should mask out the permission denied too? We're now
> displaying what information we _do_ have on those entries to stdout (the other
> ones were discarded because goto error, but the "permission denied" ones
> should), and when you request info it can't fetch it gives you ??? output.
>
> Not sure what the right behavior is here. (It's a UI question really.)

i think we're already there, aren't we? the only reason we were seeing
any errors above was because of this bug. the EACCES on init was
because that *is* a symlink so we didn't get EINVAL like the others,
but with the fix, everything looks right to me (if you assume that the
coreutils "? for stuff we don't know" is the desired behavior, which i
think is probably as close to ideal as you'll get):

~/aosp/external/toybox$ adb shell id
uid=2000(shell) gid=2000(shell)
groups=2000(shell),1004(input),1007(log),1011(adb),1015(sdcard_rw),1028(sdcard_r),3001(net_bt_admin),3002(net_bt),3003(inet),3006(net_bw_stats),3009(readproc),3011(uhid)
context=u:r:shell:s0
~/aosp/external/toybox$ adb shell ls
acct
adb_keys
apex
bin
bugreports
cache
config
d
data
debug_ramdisk
default.prop
dev
dsp
etc
init
init.environ.rc
lost+found
metadata
mnt
odm
oem
persist
postinstall
proc
product
res
sdcard
storage
sys
system
system_ext
vendor
~/aosp/external/toybox$ adb shell ls -l
total 53
dr-xr-xr-x  56 root   root       0 1970-08-05 21:58 acct
-?????????   ? ?      ?          ?                ? adb_keys
drwxr-xr-x  30 root   root     600 2019-12-13 14:03 apex
lrw-r--r--   1 root   root      11 2019-12-06 20:28 bin -> /system/bin
lrw-r--r--   1 root   root      50 2019-12-06 20:28 bugreports ->
/data/user_de/0/com.android.shell/files/bugreports
l?????????   ? ?      ?          ?                ? cache -> ?
drwxr-xr-x   4 root   root       0 1969-12-31 19:00 config
lrw-r--r--   1 root   root      17 2019-12-06 20:28 d -> /sys/kernel/debug
drwxrwx--x  51 system system  4096 2019-12-13 14:03 data
drwxr-xr-x   2 root   root    4096 2019-12-03 21:17 debug_ramdisk
lrw-------   1 root   root      23 2019-12-06 20:28 default.prop ->
system/etc/prop.default
drwxr-xr-x  21 root   root    4740 2019-12-06 20:40 dev
lrw-r--r--   1 root   root      11 2019-12-06 20:28 dsp -> /vendor/dsp
lrw-r--r--   1 root   root      11 2019-12-06 20:28 etc -> /system/etc
l?????????   ? ?      ?          ?                ? init -> ?
-?????????   ? ?      ?          ?                ? init.environ.rc
drwx------   2 root   root   16384 2019-12-06 20:28 lost+found
d?????????   ? ?      ?          ?                ? metadata
drwxr-xr-x  14 root   system   300 1970-08-05 21:58 mnt
drwxr-xr-x   2 root   root    4096 2019-12-03 21:17 odm
drwxr-xr-x   2 root   root    4096 2019-12-03 21:17 oem
lrw-r--r--   1 root   root      19 2019-12-06 20:28 persist ->
/mnt/vendor/persist
d?????????   ? ?      ?          ?                ? postinstall
dr-xr-xr-x 739 root   root       0 1969-12-31 19:00 proc
drwxr-xr-x   1 root   root    3488 2019-12-06 20:29 product
drwxr-xr-x   3 root   root    4096 2019-12-03 21:17 res
lrw-r--r--   1 root   root      21 2019-12-06 20:28 sdcard ->
/storage/self/primary
drwxr-xr-x   4 root   root      80 2019-12-06 20:40 storage
dr-xr-xr-x  12 root   root       0 1970-08-05 21:58 sys
drwxr-xr-x   1 root   root    3488 2019-12-13 14:03 system
drwxr-xr-x   2 root   root    4096 2019-12-03 21:17 system_ext
drwxr-xr-x   1 root   root    3488 2019-12-06 20:29 vendor

> Rob