Re: No extended attributes for sticky directories? (and samba)

Andreas,

On 03/11/06, Andreas Gruenbacher <[email protected]> wrote:

> Indeed, yes. The xfs code has it right, and the patch is good. I'll fix the
> two typos in the comment above in the same go, and push the resulting patch
> (attached) upstream. Thanks for your help!

That's quite exciting for me; I've taken my first open source step,
however small. Thanks to both you and Dave.

But... I'm not sure that the patch and xfs can/should both be right at
the same time.

Without the patch, the linux behaviour is the same for all
filesystems. There's no access at all to extended attributes on sticky
directories, for any user including root. Because the vfs behaviour is
more restictive than that of xfs this applies to xfs too.

The patch does two things: firstly it aligns vfs behaviour with the
documentation and xfs, by allowing access for the owner and priveleged
users. Then, it goes further, and allows read access for all users. My
grounds for this latter change were that it felt intuitively right to
me, and that this behaviour appeared to have been in the kernel in
earlier versions (although I was going only from snippets, there
appeared to have been a return in the function for the read-only case
before stickyness was tested, the sort of thing that's easy to
overlook when making changes).

Anyway, the latter change makes the vfs behaviour _less_ restrictive
than xfs, so the net effect is:

vfs (therefore jfs/ext2/ext3 and most other filesystems):
  Read: allowed for all users
  Write: owner and CAP_FOWNER only

xfs:
  Read: owner and CAP_FOWNER only
  Write: owner and CAP_FOWNER only

My thinking was that either the patch should be aligned with xfs (by
leaving out the "allow read access part") or vice versa (by changing
xfs to allow read access). My intent was to query which path to take.
I apologise for not making this clear, I didn't quite believe I'd
found a problem or it's solution, and the point got a bit lost in my
uncertainty. It simply didn't occur to me that my patch would be
accepted as is. I'll be more careful in future: beware when posting a
patch... someone might just apply it!

I don't believe the unmodified patch should be used because it would
introduce a slight difference in behaviour between xfs and all other
filesystems. That's fine if there's a good reason for such a
difference, but otherwise it should be avoided on general principles:
it can lead to unexpected, hard to track consequences. Case in point:
samba. The consequence would be that hidden files suddenly appear
unhidden if a sticky directory appears anywhere on a share with an
underlying xfs filesystem. Sure, the samba code should be improved,
but we're already in the margins with the sticky directory thing to
begin with; it's worse if it's just for xfs and for no good reason.

I guess my "preference" was to modify xfs to allow read access... but
I didn't actually go ahead include the change in the patch because of
the following: according to my reading, the code has a notion of
attribute "capability" which is relative to process credentials; if an
i-node is attribute-capable, then it is allowed to take attributes,
otherwise not. My gut feeling is that the word "capable" precludes
making it relative to the type of access, otherwise a word like
"permission" would have been used; but it's impossible to tell for
sure from the code, that's just the way I relate to English. It's easy
enough to make the change (the type of access is passed to
attr_generic_get() via xflags, but that would involve either changing
the meaning of "capable" according to my reading or a slight refactor.
In any case, I felt the whole thing needed discussion first.

Anyway, the last thing I want is to introduce problems because I
didn't make myself clear enough, so I'd appreciate it Andreas if you
could reassure me / modify the patch / consider the change to xfs.
Once again, thanks for the replies and encouragement.

Regards,

Gerard
_______________________________________________
acl-devel mailing list
[email protected]
http://acl.bestbits.at/mailman/listinfo/acl-devel