ls: fix capability coloring

Capability checking was incorrectly done on just the base name
rather than on the whole path.  Consequently there could be both
false positives and negatives when coloring files with capabilities.
Also capability checking was not done at all in certain cases for
non executable files.  Note passing absolute rather than relative
names to cap_get_file() reduces the has_capability() overhead
from around 33% to 30%.  I.E. ls --color is now around 3% faster.

* src/ls.c (struct fileinfo): Add a has_capability member.
(print_color_indicator): Refactor to pass just a fileinfo pointer
and a flag to say if we're dealing with a symlink target.
(print_name_with_quoting): Likewise.
(gobble_file): Set has_capability in the fileinfo struct.  Also do
a capability check even if executable coloring is disabled.
Ditto for SETUID and SETUID coloring.
Comment on how expensive has_capability() is.
(print_long_format): Adjust to refactored print_name_with_quoting.
(quote_name): Likewise.
(print_file_name_and_frills): Likewise.
* tests/ls/capability: Test the various false positive and negatives.
* THANKS: Add reporter (Ivan Labath).
* NEWS: Mention the fix.

1 files changed, 83 insertions, 65 deletions

diff --git a/src/ls.c b/src/ls.c
index 08fdf5fdc..aeaa58417 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -214,6 +214,9 @@ struct fileinfo
     /* For long listings, true if the file has an access control list,
        or an SELinux security context.  */
     enum acl_type acl_type;
+
+    /* For color listings, true if a regular file has capability info.  */
+    bool has_capability;
   };
 
 #define LEN_STR_PAIR(s) sizeof (s) - 1, s
@@ -241,9 +244,8 @@ static bool file_ignored (char const *name);
 static uintmax_t gobble_file (char const *name, enum filetype type,
                               ino_t inode, bool command_line_arg,
                               char const *dirname);
-static bool print_color_indicator (const char *name, mode_t mode, int linkok,
-                                   bool stat_ok, enum filetype type,
-                                   nlink_t nlink);
+static bool print_color_indicator (const struct fileinfo *f,
+                                   bool symlink_target);
 static void put_indicator (const struct bin_str *ind);
 static void add_ignore_pattern (const char *pattern);
 static void attach (char *dest, const char *dirname, const char *name);
@@ -264,11 +266,9 @@ static int format_user_width (uid_t u);
 static int format_group_width (gid_t g);
 static void print_long_format (const struct fileinfo *f);
 static void print_many_per_line (void);
-static size_t print_name_with_quoting (const char *p, mode_t mode,
-                                       int linkok, bool stat_ok,
-                                       enum filetype type,
+static size_t print_name_with_quoting (const struct fileinfo *f,
+                                       bool symlink_target,
                                        struct obstack *stack,
-                                       nlink_t nlink,
                                        size_t start_col);
 static void prep_non_filename_text (void);
 static bool print_type_indicator (bool stat_ok, mode_t mode,
@@ -2637,6 +2637,37 @@ unsigned_file_size (off_t size)
   return size + (size < 0) * ((uintmax_t) OFF_T_MAX - OFF_T_MIN + 1);
 }
 
+#ifdef HAVE_CAP
+/* Return true if NAME has a capability (see linux/capability.h) */
+static bool
+has_capability (char const *name)
+{
+  char *result;
+  bool has_cap;
+
+  cap_t cap_d = cap_get_file (name);
+  if (cap_d == NULL)
+    return false;
+
+  result = cap_to_text (cap_d, NULL);
+  cap_free (cap_d);
+  if (!result)
+    return false;
+
+  /* check if human-readable capability string is empty */
+  has_cap = !!*result;
+
+  cap_free (result);
+  return has_cap;
+}
+#else
+static bool
+has_capability (char const *name ATTRIBUTE_UNUSED)
+{
+  return false;
+}
+#endif
+
 /* Enter and remove entries in the table `cwd_file'.  */
 
 /* Empty the table of files.  */
@@ -2718,11 +2749,16 @@ gobble_file (char const *name, enum filetype type, ino_t inode,
                  to see if it's executable.  */
               || (type == normal && (indicator_style == classify
                                      /* This is so that --color ends up
-                                        highlighting files with the executable
-                                        bit set even when options like -F are
-                                        not specified.  */
+                                        highlighting files with these mode
+                                        bits set even when options like -F are
+                                        not specified.  Note we do a redundant
+                                        stat in the very unlikely case where
+                                        C_CAP is set but not the others. */
                                      || (print_with_color
-                                         && is_colored (C_EXEC))
+                                         && (is_colored (C_EXEC)
+                                             || is_colored (C_SETUID)
+                                             || is_colored (C_SETGID)
+                                             || is_colored (C_CAP)))
                                      )))))
 
     {
@@ -2793,6 +2829,11 @@ gobble_file (char const *name, enum filetype type, ino_t inode,
 
       f->stat_ok = true;
 
+      /* Note has_capability() adds around 30% runtime to `ls --color`  */
+      if ((type == normal || S_ISREG (f->stat.st_mode))
+          && print_with_color && is_colored (C_CAP))
+        f->has_capability = has_capability (absolute_name);
+
       if (format == long_format || print_scontext)
         {
           bool have_selinux = false;
@@ -3761,18 +3802,14 @@ print_long_format (const struct fileinfo *f)
     }
 
   DIRED_FPUTS (buf, stdout, p - buf);
-  size_t w = print_name_with_quoting (f->name, FILE_OR_LINK_MODE (f), f->linkok,
-                                      f->stat_ok, f->filetype, &dired_obstack,
-                                      f->stat.st_nlink, p - buf);
+  size_t w = print_name_with_quoting (f, false, &dired_obstack, p - buf);
 
   if (f->filetype == symbolic_link)
     {
       if (f->linkname)
         {
           DIRED_FPUTS_LITERAL (" -> ", stdout);
-          print_name_with_quoting (f->linkname, f->linkmode, f->linkok - 1,
-                                   f->stat_ok, f->filetype, NULL,
-                                   f->stat.st_nlink, (p - buf) + w + 4);
+          print_name_with_quoting (f, true, NULL, (p - buf) + w + 4);
           if (indicator_style != none)
             print_type_indicator (true, f->linkmode, unknown);
         }
@@ -3948,19 +3985,20 @@ quote_name (FILE *out, const char *name, struct quoting_options const *options,
 }
 
 static size_t
-print_name_with_quoting (const char *p, mode_t mode, int linkok,
-                         bool stat_ok, enum filetype type,
-                         struct obstack *stack, nlink_t nlink,
+print_name_with_quoting (const struct fileinfo *f,
+                         bool symlink_target,
+                         struct obstack *stack,
                          size_t start_col)
 {
+  const char* name = symlink_target ? f->linkname : f->name;
+
   bool used_color_this_time
-    = (print_with_color
-       && print_color_indicator (p, mode, linkok, stat_ok, type, nlink));
+    = (print_with_color && print_color_indicator (f, symlink_target));
 
   if (stack)
     PUSH_CURRENT_DIRED_POS (stack);
 
-  size_t width = quote_name (stdout, p, filename_quoting_options, NULL);
+  size_t width = quote_name (stdout, name, filename_quoting_options, NULL);
   dired_pos += width;
 
   if (stack)
@@ -4012,9 +4050,7 @@ print_file_name_and_frills (const struct fileinfo *f, size_t start_col)
   if (print_scontext)
     printf ("%*s ", format == with_commas ? 0 : scontext_width, f->scontext);
 
-  size_t width = print_name_with_quoting (f->name, FILE_OR_LINK_MODE (f),
-                                          f->linkok, f->stat_ok, f->filetype,
-                                          NULL, f->stat.st_nlink, start_col);
+  size_t width = print_name_with_quoting (f, false, NULL, start_col);
 
   if (indicator_style != none)
     width += print_type_indicator (f->stat_ok, f->stat.st_mode, f->filetype);
@@ -4065,55 +4101,38 @@ print_type_indicator (bool stat_ok, mode_t mode, enum filetype type)
   return !!c;
 }
 
-#ifdef HAVE_CAP
-/* Return true if NAME has a capability (see linux/capability.h) */
-static bool
-has_capability (char const *name)
-{
-  char *result;
-  bool has_cap;
-
-  cap_t cap_d = cap_get_file (name);
-  if (cap_d == NULL)
-    return false;
-
-  result = cap_to_text (cap_d, NULL);
-  cap_free (cap_d);
-  if (!result)
-    return false;
-
-  /* check if human-readable capability string is empty */
-  has_cap = !!*result;
-
-  cap_free (result);
-  return has_cap;
-}
-#else
-static bool
-has_capability (char const *name ATTRIBUTE_UNUSED)
-{
-  return false;
-}
-#endif
-
 /* Returns whether any color sequence was printed. */
 static bool
-print_color_indicator (const char *name, mode_t mode, int linkok,
-                       bool stat_ok, enum filetype filetype,
-                       nlink_t nlink)
+print_color_indicator (const struct fileinfo *f, bool symlink_target)
 {
   enum indicator_no type;
   struct color_ext_type *ext;	/* Color extension */
   size_t len;			/* Length of name */
 
+  const char* name;
+  mode_t mode;
+  int linkok;
+  if (symlink_target)
+    {
+      name = f->linkname;
+      mode = f->linkmode;
+      linkok = f->linkok - 1;
+    }
+  else
+    {
+      name = f->name;
+      mode = FILE_OR_LINK_MODE (f);
+      linkok = f->linkok;
+    }
+
   /* Is this a nonexistent file?  If so, linkok == -1.  */
 
   if (linkok == -1 && color_indicator[C_MISSING].string != NULL)
     type = C_MISSING;
-  else if (! stat_ok)
+  else if (!f->stat_ok)
     {
       static enum indicator_no filetype_indicator[] = FILETYPE_INDICATORS;
-      type = filetype_indicator[filetype];
+      type = filetype_indicator[f->filetype];
     }
   else
     {
@@ -4125,12 +4144,11 @@ print_color_indicator (const char *name, mode_t mode, int linkok,
             type = C_SETUID;
           else if ((mode & S_ISGID) != 0 && is_colored (C_SETGID))
             type = C_SETGID;
-          /* has_capability() called second for performance.  */
-          else if (is_colored (C_CAP) && has_capability (name))
+          else if (is_colored (C_CAP) && f->has_capability)
             type = C_CAP;
           else if ((mode & S_IXUGO) != 0 && is_colored (C_EXEC))
             type = C_EXEC;
-          else if ((1 < nlink) && is_colored (C_MULTIHARDLINK))
+          else if ((1 < f->stat.st_nlink) && is_colored (C_MULTIHARDLINK))
             type = C_MULTIHARDLINK;
         }
       else if (S_ISDIR (mode))