On Sat, Oct 17, 2015 at 9:21 AM, Rich Felker <[email protected]> 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@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. _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
