Bug 692982

+++ This bug was initially created as a clone of Bug #692823 +++

Description of problem:

/bin/ls -l on an automount directory of some nature peforms the following ops on each file:

lstat("doc", {st_mode=S_IFDIR|S_ISGID|0775, st_size=4096, ...}) = 0
lgetxattr("doc", "security.selinux", "unconfined_u:object_r:default_t:s0", 255) = 35
getxattr("doc", "system.posix_acl_access", 0x0, 0) = -1 EOPNOTSUPP (Operation not supported)

and possibly also:

getxattr("/etc", "system.posix_acl_default", 0x0, 0) = -1 ENODATA (No data available)

depending on the circumstances.

The first two calls (lstat and lgetxattr) don't follow symlinks and mounts, but the second is a getxattr call which _does_ follow symlinks and mounts.  The latter can cause automount points to mount, which the former won't, at least from linux-2.6.38-rc1 onwards.

These should be consistent, and so I believe the getxattr calls should be lgetxattr calls.  Without this, the first two calls may refer to a different object to the second pair of calls.

Version-Release number of selected component (if applicable):

F-13 and F-14 coreutils both show this.  coreutils-8.5-7.fc14.x86_64 is what's on my desktop.

How reproducible:

100%

Steps to Reproduce:
1. strace -e trace=lstat,getxattr,lgetxattr ls -l /media
  
Actual results:

lstat("/media", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lgetxattr("/media", "security.selinux", "system_u:object_r:mnt_t:s0", 255) = 27
getxattr("/media", "system.posix_acl_access", 0x0, 0) = -1 ENODATA (No data available)
getxattr("/media", "system.posix_acl_default", 0x0, 0) = -1 ENODATA (No data available)

Expected results:

The getxattrs should be lgetxattrs.

Additional info:

This is a problem that occurs in conjunction with the acl package - that doesn't provide a way of calling lgetxattr() through it.

--- Additional comment from ovasik on 2011-04-01 12:25:24 CEST ---

coreutils (gnulib) just uses acl_extended_file from libacl - which actually does the getxattr calls. Reassigning to acl package.

--- Additional comment from kdudka on 2011-04-01 12:46:46 CEST ---

There is not much we can do with acl_extended_file().  Symlinks are always followed when setting/getting ACLs, because they can have no ACLs themselves.  The only choice I see is to either not call acl_extended_file() for symlinks, or use acl_extended_fd(), which is not applicable on symlinks by definition.

--- Additional comment from ovasik on 2011-04-01 13:16:10 CEST ---

File_has_acl() gnulib function already calls libacl acl_extended_file() only for non-symlinks ...
But as discussed of bugzilla, it really looks it can't be fixed in libacl and coreutils is probably the right place.

--- Additional comment from dhowells on 2011-04-01 15:48:48 CEST ---

The files in question are not symlinks; they are directories with follow_link semantics in the kernel (or the equivalent in 2.6.38+).

You also cannot simply look at the filesystem the target file is on and ask "is it autofs?".  The filesystem may be nfs4 (referrals), cifs (DFS junction point) or afs (volume mount point), and more may follow.  Currently, you cannot tell if it's an automount point or not.

I would think there needs to be a version of acl_extended_file() that calls lgetxattr().  If that doesn't work because the file is a symlink and thus has no ACL, then ls can use acl_extended_file() if -L was passed to it or if it's going to dereference it anyway.

--- Additional comment from dhowells on 2011-04-01 16:56:23 CEST ---

> There is no call of l?etxattr() in the whole acl library.  Does any other ACL
> implementation have such ability?

MacOS X's getxattr() can take a XATTR_NOFOLLOW flag that has the same effect as using lgetxattr().

Also, isn't acl_extended_file() a Linux extension?  Would it be that hard to add an acl_extended_file_nofollow() or something?

--- Additional comment from kdudka on 2011-04-01 17:39:48 CEST ---

(In reply to comment #7)
> Also, isn't acl_extended_file() a Linux extension?  Would it be that hard to
> add an acl_extended_file_nofollow() or something?

Andreas, is addition of such a new function to libacl acceptable by upstream?
The problem here is that acl_extended_file() triggers a mount even when not necessary.

--- Additional comment from agruen on 2011-04-01 19:35:51 CEST ---

I don't see what we can do to fix this other than not using acl_extended_file() in file_has_acl().  We could either add and use acl_extended_file_nofollow() there or put that code directly into gnulib; the code required is quite trivial and just as Linux specific as acl_extended_file().

Which of the two would you prefer?

--- Additional comment from kdudka on 2011-04-01 20:01:01 CEST ---

I'd prefer to create a new function in libacl such that we maintain all the code aware of the acl->xattr mapping at a single place.  If such a new function is acceptable, I'll send a patch to acl-devel.

--- Additional comment from agruen on 2011-04-01 23:02:17 CEST ---

That should work, thanks.

I'm adding Jim; this should be relevant for him, too.