Ok, here's a patch that seems to take care of all the problems... I am
not an acl-guru by -any- means though, so I'd appreciate any testing &
sanity-checking. I'll ask the sgi acl-gurus to look at it as well.
(I'm not very happy with the test -x fix, it seems like this should be
much earlier - perhaps even the vfs could just take care of this for
us...)
-Eric
On Sat, 2002-10-12 at 15:43, Ethan Benson wrote:
> On Sat, Oct 12, 2002 at 05:30:26PM +0200, Axel Thimm wrote:
> > The bug seems to be in the acl parts and is not restricted to root
> > XFS-partitions (Eric, sorry for my bad wording about "root-mounted"
> > partitions
> > in the previous mail).
> >
> > It is also difficult to trigger - creating files is fine, copying them with
> > "cp -p" to a XFS-partition (src is not relevant) is creating an (empty?) acl
> > entry, which "test -x" misinterpretes if you are root (these acls also
> > cannot
> > be removed with setfacl -b and setfacl -k, and are also not diplayed with
> > getfacl or getfattr).
>
> yes this is exceedingly annoying since acl aware ls shows such files
> with a + on the permissions, which is erroneous.
>
> you can see the acls with the following:
>
> getfattr -m . file
>
> you will see a system.posix_acl_access attribute (system.* is not
> displayed by default).
>
> you can remove them with:
>
> setfattr -x system.posix_acl_access file
>
> > In general the bugs seem to be described as:
> > o acl aware applications like "cp -p" create empty unremovable acls (or set
> > some acl-is-here flag without truly holding an acl).
>
> the problem is XFS is not removing acl xattrs when they contain
> nothing that can't be stored in the inode mode feild. this also
> affects directories with basic default acls (which only, say turn on
> the group write permission, all new files should, and do have only basic
> unix permissions, but still have a acl xattr making things like ls
> believe there is an acl when there isn't one).
>
> i propose this be fixed by either adding a check to see if the acl has
> anything more then the basic unix permissions, and if so dispose with
> creating an xattr. and/or in the function which syncs the inode mode
> bits with the acl's basic entries, if the acl does not contain any
> extended ACEs then delete the acl attribute. fwiw SunOS with acl
> support does not exhibit these problems.
>
> > o files with acls (empty or not) are always interpreted to have te
> > permission
> > flag set if you are root.
>
> s/te/the execute/ i presume.
>
> yes thats a separate bug.
>
> > BTW there is a small name clash between vanilla RedHat and SGI provided acl
> > rpms: What SGI calls acl-devel, RedHat decided to call libacl-devel. But I
> > think this is only a package dependency problem (the XFS acl-devel should
> > obsolete libacl-devel or should adjust the name).
>
> libacl is the correct name. this is already planned to be adjusted
> for at least debian which has strict (and quite sane) policies on the
> naming of these sorts of packages.
>
> > /var/testing-XFS-acl-2cp-simple OK
> > /var/testing-XFS-acl-3cp-p BUG: test -x returns true
> > /var/testing-XFS-acl-4chacl BUG: test -x returns true
> > /var/testing-XFS-acl-5nonremovableacls BUG: test -x returns true
> > /var/testing-XFS-acl-6awithtrueacls BUG: test -x returns true
> > /var/testing-XFS-acl-6bwithtrueaclsthenremoved BUG: test -x returns true
>
> can you try this test by removing the acl with the above setfattr
> command instead of setfacl (which seems to be partly broken in regards
> to acl removal)
>
> > # ==========> all but /var/testing-XFS-acl-6awithtrueacls should not have a
> > + at the mods
> > -rw-r--r-- 1 root root 0 Oct 12 17:15
> > /var/testing-XFS-acl-1touched
> > -rw-r--r-- 1 root root 0 Oct 12 17:15
> > /var/testing-XFS-acl-2cp-simple
> > -rw-r--r--+ 1 root root 0 Oct 12 17:15
> > /var/testing-XFS-acl-3cp-p
> > -rw-r--r--+ 1 root root 0 Oct 12 17:15
> > /var/testing-XFS-acl-4chacl
> > -rw-r--r--+ 1 root root 0 Oct 12 17:15
> > /var/testing-XFS-acl-5nonremovableacls
> > -rw-r--r--+ 1 root root 0 Oct 12 17:15
> > /var/testing-XFS-acl-6awithtrueacls
> > -rw-r--r--+ 1 root root 0 Oct 12 17:15
> > /var/testing-XFS-acl-6bwithtrueaclsthenremoved
>
> notice all the + that should not be there.
>
> > -rw-r--r-- 1 thimm ag-linke 0 Oct 12 17:15
> > /var/testing-XFS-acl-1touched
> > -rw-r--r-- 1 thimm ag-linke 0 Oct 12 17:15
> > /var/testing-XFS-acl-2cp-simple
> > -rw-r--r--+ 1 thimm ag-linke 0 Oct 12 17:15
> > /var/testing-XFS-acl-3cp-p
> > -rw-r--r--+ 1 thimm ag-linke 0 Oct 12 17:15
> > /var/testing-XFS-acl-4chacl
> > -rw-r--r--+ 1 thimm ag-linke 0 Oct 12 17:15
> > /var/testing-XFS-acl-5nonremovableacls
> > -rw-r--r--+ 1 thimm ag-linke 0 Oct 12 17:15
> > /var/testing-XFS-acl-6awithtrueacls
> > -rw-r--r--+ 1 thimm ag-linke 0 Oct 12 17:15
> > /var/testing-XFS-acl-6bwithtrueaclsthenremoved
>
> still a bug, only one of those should have a +
>
> > # ==========> only /var/testing-XFS-acl-6awithtrueacls should be listed
> > # file: /var/testing-XFS-acl-6awithtrueacls
> > # owner: thimm
> > # group: ag-linke
> > user::rw-
> > user:nobody:r--
> > group::r--
> > mask::r--
> > other::r--
>
> --
> Ethan Benson
> http://www.alaska.net/~erbenson/
--
Eric Sandeen XFS for Linux http://oss.sgi.com/projects/xfs
sandeen@xxxxxxx SGI, Inc. 651-683-3102
xfs_acl.c.patch.gz
Description: GNU Zip compressed data
|