Bugzilla – Bug 790
getfacl 2.2.47 follows symlinks, even without -L
Last modified: 2009-12-23 13:53:20 CST
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...
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.
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.
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.
Created attachment 247 [details] Prevent errors for dead symlinks that we're not touching anyway
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".
(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