[Toybox] ls -laZ /dev takes a long time

[enh]

On Sat, Oct 17, 2015 at 9:21 AM, Rich Felker <dalias at libc.org> wrote:
> On Fri, Oct 16, 2015 at 12:56:01PM -0700, enh wrote:
>> at the moment, ls.c has this:
>>
>>   if (flags & FLAG_Z) {
>>     if (!CFG_TOYBOX_LSM_NONE) {
>>       int fd;
>>
>>       // Why not just openat(O_PATH|(O_NOFOLLOW*!!(toys.optflags&FLAG_L))) and
>>       // lsm_fget_context() on that filehandle? Because the kernel is broken,
>>       // and won't let us read this "metadata" from the filehandle unless we
>>       // have permission to read the data. We _can_ read the same data in
>>       // by path, we just can't do it through an O_PATH filehandle, because
>>       // reasons. So as a bug workaround for the broken kernel, we do it
>>       // both ways.
>>       //
>>       // The O_NONBLOCK is there to avoid triggering automounting (there's
>>       // a rush of nostalgia for you) on directories we don't descend into,
>>       // which O_PATH would have done for us but see "the kernel is broken".
>>       if (S_ISSOCK(new->st.st_mode) ||
>>           (S_ISLNK(new->st.st_mode) && !(toys.optflags & FLAG_L)) ||
>>           -1 == (fd = openat(dirtree_parentfd(new), new->name,
>>                              O_RDONLY|O_NONBLOCK|O_NOATIME)))
>
> I don't know about this specific case, but generally you can work
> around all the situations where O_PATH misbehaves when operating on
> the resulting fd by using the function that takes a pathname and
> passing /proc/self/fd/%d with the O_PATH fd filled-in instead.

yeah, that's what bionic's *xattr functions do (which was apparently
the only part of the linked-to patch rob didn't look at :-) --- the
direct link to that file is
https://android-review.googlesource.com/#/c/152663/4/libc/bionic/fgetxattr.cpp
).

that was what i intended as option 2: basically copy that code into
toybox. (option 1 being, as rob guessed, if (CFG_ANDROID)
rely-on-bionic else the-current-code.)

>> but that doesn't work well in /dev.
>>
>> root at shamu:/ # time ls -laZ /dev > /dev/null
>>     0m10.33s real     0m0.05s user     0m0.62s system
>>
>> some files, like /dev/smd_pkt_loopback, take a long time to fail to open:
>>
>>      0.003022 openat(4, "smd_pkt_loopback",
>> O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_NOATIME) = -1 EPROBE_DEFER (Unknown
>> error 517)
>>      5.185825 lgetxattr("/dev/smd_pkt_loopback", "security.selinux",
>> "u:object_r:device:s0", 255) = 21
>>
>> it seems dangerous to be opening every file when running "ls", as
>> opening a /dev file could have side effects or hang. if an open is
>
> Yes, unwanted opening of device nodes is generally dangerous. It can
> also cause side effects like acquiring a controlling terminal; this
> specific one can be avoided with O_NOCTTY but others probably exist
> too.
>
> Rich



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.

 1445104047.0