[Toybox] ls -Z with relative paths

enh enh at google.com
Sat Jan 9 13:38:02 PST 2016


i have the following internal bug:

ls -laZ doesn't properly return the SELinux label for files where a
non-fully qualified path is provided.

Steps to reproduce (on Nexus 9):

  adb shell ls -laZ /sys/kernel/debug/tracing/tracing_on
  adb shell ls -laZ /sys/kernel/debug/tracing/ | grep tracing_on
  adb shell 'cd /sys/kernel/debug/tracing ; ls -laZ tracing_on'

Expected: The output of all commands should yield the same SELinux
label: "u:object_r:debugfs_tracing:s0"

Actual:

The third command returns "?" for the SELinux label.

  $ adb shell ls -laZ /sys/kernel/debug/tracing/tracing_on
  -rw-rw-r-- 1 root shell u:object_r:debugfs_tracing:s0 0 1970-01-01
00:00 /sys/kernel/debug/tracing/tracing_on
  $ adb shell ls -laZ /sys/kernel/debug/tracing/ | grep tracing_on
  -rw-rw-r--  1 root shell u:object_r:debugfs_tracing:s0      0
1970-01-01 00:00 tracing_on
  $ adb shell 'cd /sys/kernel/debug/tracing ; ls -laZ tracing_on'
  -rw-rw-r-- 1 root shell ? 0 1970-01-01 00:00 tracing_on


looking at strace, the problem is that we're getting 0 from dirtree_parentfd:

strace of command 1:

newfstatat(AT_FDCWD, "/sys/kernel/debug/tracing/tracing_on",
{st_mode=S_IFREG|0664, st_size=0, ...}, AT_SYMLINK_NOFOLLOW) = 0
openat(0, "/sys/kernel/debug/tracing/tracing_on",
O_RDONLY|O_NOFOLLOW|O_PATH) = 3
fgetxattr(3, "security.selinux", 0x7fb3a3d000, 255) = -1 EBADF (Bad
file descriptor)
fcntl(3, F_GETFL)                       = 0x208000 (flags
O_RDONLY|O_NOFOLLOW|O_PATH)
getxattr("/proc/self/fd/3", "security.selinux",
"u:object_r:debugfs_tracing:s0", 255) = 30
close(3)                                = 0

strace of command 3:

newfstatat(AT_FDCWD, "tracing_on", {st_mode=S_IFREG|0664, st_size=0,
...}, AT_SYMLINK_NOFOLLOW) = 0
openat(0, "tracing_on", O_RDONLY|O_NOFOLLOW|O_PATH) = -1 ENOTDIR (Not
a directory)

this is fine for absolute paths because the inappropriate fd never
gets used, but it breaks ls -Z for relative paths.

a hack like this shows that everything else is right:

diff --git a/toys/posix/ls.c b/toys/posix/ls.c
index 2f44d24..3699693 100644
--- a/toys/posix/ls.c
+++ b/toys/posix/ls.c
@@ -191,7 +191,9 @@ static int filter(struct dirtree *new)
       // (Wouldn't it be nice if the lsm functions worked like openat(),
       // fchmodat(), mknodat(), readlinkat() so we could do this without
       // even O_PATH? But no, this is 1990's tech.)
-      int fd = openat(dirtree_parentfd(new), new->name,
+      int parent_fd = dirtree_parentfd(new);
+      if (parent_fd == 0) parent_fd = AT_FDCWD;
+      int fd = openat(parent_fd, new->name,
         O_PATH|(O_NOFOLLOW*!(toys.optflags&FLAG_L)));

       if (fd != -1) {

but it's not clear to me what the correct fix is.

something like this seems sensible for ls:

diff --git a/toys/posix/ls.c b/toys/posix/ls.c
index 2f44d24..08ae695 100644
--- a/toys/posix/ls.c
+++ b/toys/posix/ls.c
@@ -541,6 +541,7 @@ void ls_main(void)
   // Iterate through command line arguments, collecting directories and files.
   // Non-absolute paths are relative to current directory.
   TT.files = dirtree_start(0, 0);
+  TT.files->dirfd = AT_FDCWD;
   for (s = *toys.optargs ? toys.optargs : noargs; *s; s++) {
     dt = dirtree_start(*s, !(toys.optflags&(FLAG_l|FLAG_d|FLAG_F)) ||
                             (toys.optflags&(FLAG_L|FLAG_H)));

but i'm not sure whether the semantics of dirtree_start are supposed
to imply this? certainly defaulting dirfd to -1 rather than 0 might
lead to fewer surprises in future.

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



More information about the Toybox mailing list