Re: [PATCH] file-has-acl: use acl_extended_file_nofollow() if available

[Jim Meyering]

Kamil Dudka wrote:
> On Wednesday 06 April 2011 16:49:31 Kamil Dudka wrote:
>> the attached patch allows ls(1) from coreutils to eliminate unnecessary
>> calls of getxattr() and thus prevents it from triggerring unnecessary
>> mounts while listing files.  The feature is enabled automatically whenever
>> acl_extended_file_nofollow() is available at build-time.  See the following
>> bug for more details:
>>
>> https://bugzilla.redhat.com/692823
>>
>> Thanks in advance for considering the patch!
>
> Attached is a new version with a typo fixed (spotted by Ondrej Vasik).

Thanks, Kamil.
Here's what I'm about to push.
I've tweaked the commit message, adjusted the added comment to mention
the race condition and reduced indentation of the added cpp directives
so that this file passes 'make check's cppi test.

I also ensured that the new test does detect/use
acl_extended_file_nofollow on Fedora 15 and that all of
the existing tests still pass.  It would be nice to add a
test for this (strace-based -- not automount-based)


>From 95f7c57ff4090a5dee062044d2c7b99879077808 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <address@hidden>
Date: Fri, 22 Jul 2011 14:48:42 +0200
Subject: [PATCH] file-has-acl: use acl_extended_file_nofollow if available

* lib/acl-internal.h (HAVE_ACL_EXTENDED_FILE): New macro.
(acl_extended_file): New macro.
* lib/file-has-acl.c (file_has_acl): Use acl_extended_file_nofollow.
* m4/acl.m4 (gl_FUNC_ACL): Check for acl_extended_file_nofollow.
This addresses http://bugzilla.redhat.com/692823.
---
 ChangeLog          |    8 ++++++++
 lib/acl-internal.h |    6 ++++++
 lib/file-has-acl.c |   10 +++++++++-
 m4/acl.m4          |    2 +-
 4 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 606b9fe..4b9955c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2011-07-22  Kamil Dudka  <address@hidden>
+
+       file-has-acl: use acl_extended_file_nofollow if available
+       * lib/acl-internal.h (HAVE_ACL_EXTENDED_FILE): New macro.
+       (acl_extended_file): New macro.
+       * lib/file-has-acl.c (file_has_acl): Use acl_extended_file_nofollow.
+       * m4/acl.m4 (gl_FUNC_ACL): Check for acl_extended_file_nofollow.
+
 2011-07-21  Bruno Haible  <address@hidden>

        Declare system functions in a way that works with C++.
diff --git a/lib/acl-internal.h b/lib/acl-internal.h
index b3160a7..b509666 100644
--- a/lib/acl-internal.h
+++ b/lib/acl-internal.h
@@ -133,6 +133,12 @@ rpl_acl_set_fd (int fd, acl_t acl)
 #  endif

 /* Linux-specific */
+#  ifndef HAVE_ACL_EXTENDED_FILE_NOFOLLOW
+#   define HAVE_ACL_EXTENDED_FILE_NOFOLLOW false
+#   define acl_extended_file_nofollow(name) (-1)
+#  endif
+
+/* Linux-specific */
 #  ifndef HAVE_ACL_FROM_MODE
 #   define HAVE_ACL_FROM_MODE false
 #   define acl_from_mode(mode) (NULL)
diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
index 3d4d5c1..2ee6ba2 100644
--- a/lib/file-has-acl.c
+++ b/lib/file-has-acl.c
@@ -366,12 +366,20 @@ file_has_acl (char const *name, struct stat const *sb)
       /* Linux, FreeBSD, MacOS X, IRIX, Tru64 */
       int ret;

-      if (HAVE_ACL_EXTENDED_FILE) /* Linux */
+      if (HAVE_ACL_EXTENDED_FILE || HAVE_ACL_EXTENDED_FILE_NOFOLLOW) /* Linux 
*/
         {
+#  if HAVE_ACL_EXTENDED_FILE_NOFOLLOW
+          /* acl_extended_file_nofollow() uses lgetxattr() in order to prevent
+             unnecessary mounts, but it returns the same result as we already
+             know that NAME is not a symbolic link at this point (modulo the
+             TOCTTOU race condition).  */
+          ret = acl_extended_file_nofollow (name);
+#  else
           /* On Linux, acl_extended_file is an optimized function: It only
              makes two calls to getxattr(), one for ACL_TYPE_ACCESS, one for
              ACL_TYPE_DEFAULT.  */
           ret = acl_extended_file (name);
+#  endif
         }
       else /* FreeBSD, MacOS X, IRIX, Tru64 */
         {
diff --git a/m4/acl.m4 b/m4/acl.m4
index d6a448a..ecf0384 100644
--- a/m4/acl.m4
+++ b/m4/acl.m4
@@ -33,7 +33,7 @@ AC_DEFUN([gl_FUNC_ACL],
            AC_CHECK_FUNCS(
              [acl_get_file acl_get_fd acl_set_file acl_set_fd \
               acl_free acl_from_mode acl_from_text \
-              acl_delete_def_file acl_extended_file \
+              acl_delete_def_file acl_extended_file acl_extended_file_nofollow 
\
               acl_delete_fd_np acl_delete_file_np \
               acl_copy_ext_native acl_create_entry_np \
               acl_to_short_text acl_free_text])
--
1.7.6.586.g302e6