Bug 790 - getfacl 2.2.47 follows symlinks, even without -L
: getfacl 2.2.47 follows symlinks, even without -L
Status: RESOLVED FIXED
Product: XFS
Classification: Unclassified
Component: xfsprogs
: Current
: PC Linux
: P2 normal
: ---
Assigned To: XFS power people
:
:
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-18 02:18 CDT by Johan Ymerson
Modified: 2014-02-02 20:27 CST (History)
3 users (show)

See Also:


Attachments
Prevent errors for dead symlinks that we're not touching anyway (1.70 KB, patch)
2008-11-06 03:41 CST, Matthijs Kooijman
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Johan Ymerson 2008-09-18 02:18:32 CDT
The current version of getfacl (2.2.47) seems to ignore the physical option. It
always follow all symbolic links.

For example:
sesr10 /# getfacl --skip-base -RP files/home
getfacl: files/home/johana/.wine/dosdevices/z:/usr/lib64/klibc/include/asm: No
such file or directory
getfacl: files/home/johana/.wine/dosdevices/z:/usr/lib/klibc/include/asm: No
such file or directory
getfacl: files/home/johana/.wine/dosdevices/z:/home/joga: Permission denied
...

z: is a symbolic link to / created by Wine...
Comment 1 Matthijs Kooijman 2008-11-06 01:30:52 CST
It seems that setfacl also ignores this option (or rather, follows symlinks even
when -L is not specified, -P only applies to symlinks specified on the
commandline), which can be rather nasty.

-P does work properly without -R.
Comment 2 Matthijs Kooijman 2008-11-06 02:25:11 CST
This bug has been fixed already in CVS
(http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-cmds/acl/libmisc/walk_tree.c).

Any chance of releasing this fix anytime soon? IMHO it's quite a critical bug,
since it can mess up carefully planned permissions easily.
Comment 3 Matthijs Kooijman 2008-11-06 03:38:15 CST
On second thought, the fix is still not complete. The new version now correctly
refrains from touching and traversing the symlink, but it still stats it (not
just lstat).

This results in errors for broken symlinks, which are annoying and confusing.

$ mkdir foo; ln -s /nonexistent foo/bar
$ getfacl -R foo/
[snip output]
getfacl: foo//bar: No such file or directory
$ cd foo;
$ getfacl -P bar 
getfacl: bar: No such file or directory


The cause for this is that walk_tree_rec always stats a symlink, then sets the
WALK_TREE_SYMLINK flag, which is then used by both do_set (in setfacl) and
do_print (in getfacl) to ignore the file.

The attached patch fixe this by making do_set and do_print first check for
symlinks that don't need to be followed and making a quick exit, and then print
an error afterwards.

IMHO this patch is still quite hacky, and the stat on the symlink should never
happen, nor should do_print or do_set be called on symlinks that are not
considered. However, this approach is slightly more flexible for future changes,
perhaps.
Comment 4 Matthijs Kooijman 2008-11-06 03:41:30 CST
Created attachment 247 [details]
Prevent errors for dead symlinks that we're not touching anyway
Comment 5 Andreas Gruenbacher 2009-06-22 17:33:20 CDT
Thanks for the bug report and proposed patch. I've fixed it in a slightly less hackish way in this commit:

http://git.savannah.gnu.org/cgit/acl.git/commit/?id=63451a0

commit 63451a06b7484d220750ed8574d3ee84e156daf5
Author: Andreas Gruenbacher <agruen@suse.de>
Date:   Tue Jun 23 00:29:45 2009 +0200

    Make sure that getfacl -R only calls stat(2) on symlinks when it needs to

    This fixes http://oss.sgi.com/bugzilla/show_bug.cgi?id=790
    "getfacl follows symlinks, even without -L".
Comment 6 Brandon Philips 2009-12-23 13:53:20 CST
(In reply to comment #5)
> Thanks for the bug report and proposed patch. I've fixed it in a slightly less
> hackish way in this commit:
> 
> http://git.savannah.gnu.org/cgit/acl.git/commit/?id=63451a0

This fix is available in 2.2.49. See the new upstream repo releases at:
 https://savannah.nongnu.org/files/?group=acl
Comment 7 Alexa 2014-02-02 20:27:24 CST
*** Bug 260998 has been marked as a duplicate of this bug. ***
Seen live from the domain http://volichat.com/adult-chat-rooms
Marked for reference. Resolved as fixed @bugzilla.