Bug 720325

This request was evaluated by Red Hat Product Management for
inclusion in the current release of Red Hat Enterprise Linux.
Because the affected component is not scheduled to be updated
in the current release, Red Hat is unfortunately unable to
address this request at this time. Red Hat invites you to
ask your support representative to propose this request, if
appropriate and relevant, in the next release of Red Hat
Enterprise Linux. If you would like it considered as an
exception in the current release, please ask your support
representative.

upstream commit:

http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=95f7c57

coreutils needs to be rebuilt against libacl-devel >= 2.2.49-6:

checking for acl_extended_file... yes
checking for acl_extended_file_nofollow... no

spotted while examining bug #720318 comment #9

    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
Prior to this update, the acl_extended_file() function could cause unnecessary mounts of autofs when using the ls command on a directory with autofs mounted. This update adds a new acl function, acl_extended_file_nofollow(), to prevent unnecessary autofs mounts.

I would prefer respin as well, moreover, I can confirm that attached patch resolves acl issue mentioned 11607#c28 and, at the same time, keeps coreutils selftest passing (however I noticed that some tests are SKIPPED).

Created attachment 525910 [details]
fixup for the regression caused by the original patch V2

Comment on attachment 525910 [details]
fixup for the regression caused by the original patch V2

Thanks for the patch Kamil, looks almost sane, only #if HAVE_ACL_EXTENDED_FILE_NOFOLLOW should be # if HAVE_ACL_EXTENDED_FILE_NOFOLLOW - for the coding style conformance.

Indeed, will reflect that in the upstream submission next week.

(In reply to comment #20)
> Indeed, will reflect that in the upstream submission next week.

Also the acl_extended_file_wrap function should be declared static...

sent upstream:

http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28538

Ian, do we need to treat symlinks to mount points anyhow specially in this fix?  Or is it just fine to issue getxattr syscalls on them?

There was a discussion about them on the upstream mailing list:

http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28538/focus=28555

(In reply to comment #23)
> Ian, do we need to treat symlinks to mount points anyhow specially in this fix?
>  Or is it just fine to issue getxattr syscalls on them?

I don't think so because the semantic behaviour is (was) supposed
to follow that of symlinks. So any time there's a follow implied
by a system call an automount should be transparently (to user
space) mounted. 

> 
> There was a discussion about them on the upstream mailing list:
> 
> http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28538/focus=28555

>> But still still does not fix the problem of case f). Here, with your current
>> patch, "ls -lLd symlink-to-mountpoint" will call acl_extended_file(), which
>> will call getxattr(), which will activate an autofs mount.

Which, with the intended semantics, is what I think should
happen. In this case causing everything within the directory
to be mounted would be considered a deliberate request by the
user.

>> IMO, there are two solutions to this:
>>  - Either convince the kernel people to introduce a different flag, keeping
>>    LOOKUP_FOLLOW for symlinks only,

There has been some spirited debate over this change upstream
recently related to the overloading of the LOOKUP_FOLLOW flag.

After some opposition we ended up with a LOOKUP_AUTOMOUNT flag
but, because it was felt that changing semantic behaviour would
be too problematic for user space (even though this is the only
case we have seen in over six months), the semantics have been
changed back to what they were previously from 3.1.0 (which is
obviously too late for RHEL-6.2).

I still believe the semantic behaviour change is the right
choice for automounting and will, at some point, be looking
to change it back again. But that will mean addition of things
like IS_AUTO(mode) etc. as well as related package modifications
and overcoming the existing opposition to the change.

Sorry for the inconvenience this has caused.
Ian

> Well, symlinks to mount points were not mentioned in that bug.  I would need 
> to ask kernel guys whether symlinks to mount points are worth to consider in 
> this fix at all.

I don't think this is really a consideration.

The point of the change was to be able to long list directory
contents of an automount managed directory. In that case
stat()ing a symlink to a mount point a mount would occur which
is what should happen IMHO.

The directory listing case should consistently use the nofollow
variants of system calls and it was agreed that behaviour was
the right way to do it in the beginning of this bug.

>   1) understand better which file system operations triggered the autofs
>      mounts, I can't really beleive that calling getxattr on a non-symlink
>      will trigger autofs mounts.

Ummm ... what!

If it isn't a symlink then it might be a directory and it might
be an automount point. So IMO it should be mounted so the caller
gets the information of the file system it is expecting without
it changing under it.

This is the no-win situation that has plagued us for a long
time. For example,

With the semantics that I believe are needed:
getxattr(), followed by open(), followed by getxattr() will
return the same attributes for both getxattr() calls (good
behaviour).

With the pre-existing semantics:
getxattr(), followed by open(), followed by getxattr() can
return different attributes for getxattr() (bad behaviour).

With either semantics:
lgetxattr(), followed by open(), followed by lgetxattr() can
return different attributes for getxattr() (bad unavoidable
behaviour).

So the issues go on and on. My real goal, and the only
realistic one, is to minimize the bad behaviour since we can't
just always automount every time we hit a mount point due to
mount storm type problems.

Ian

(In reply to comment #26)
> 
> With either semantics:
> lgetxattr(), followed by open(), followed by lgetxattr() can
> return different attributes for getxattr() (bad unavoidable
> behaviour).

Those last two lines should read:
return different attributes for lgetxattr() (bad unavoidable
behaviour).

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2011-1693.html