Re: [PATCH] tmpfs: listxattr should include POSIX ACL xattrs

["Andreas Grünbacher" <andreas.gruenbacher@gmail.com>]

2015-11-09 0:37 GMT+01:00 Hugh Dickins <hughd@google.com>:
> On Tue, 3 Nov 2015, Andreas Gruenbacher wrote:
>
>> When a file on tmpfs has an ACL or a Default ACL, listxattr should include the
>> corresponding xattr names.
>>
>> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
>> ---
>>  fs/kernfs/inode.c     |  2 +-
>>  fs/xattr.c            | 53 +++++++++++++++++++++++++++++++++++----------------
>>  include/linux/xattr.h |  2 +-
>>  mm/shmem.c            |  2 +-
>>  4 files changed, 40 insertions(+), 19 deletions(-)
>
> Hmm, can you make a stronger argument for this patch than above?
>
> My ignorance of ACLs and XATTRs is boundless, I'll have to defer to
> you and others.

That shouldn't be a problem.

> But when I read the listxattr(2) manpage saying
> "Filesystems like ext2, ext3 and XFS which implement POSIX ACLs
> using extended attributes, might return a list like ...",
> I don't see that as mandating that any filesystem which happens
> for its own internal convenience to implement ACLs via XATTRs,
> has to list the ACLs with the XATTRs - I read it rather as an
> apology that some of them (for their own simplicity) do so.

The user-space interface for POSIX ACLs is always the same no matter
how they are implemented in the kernel: they are exposed as two
extended attributes called "system.posix_acl_access" (directories and
non-directories) and "system.posix_acl_default" (diectories only). The
extended attribute syscalls are [fl]getxattr, [fl]setxattr,
[fl]listxattr, and [fl]removexattr.

The listxattr syscalls is expected to list all the attributes which
the calling process is allowed to see / access. Tmpfs misbehaves in
this regard; it doesn't list the extended attributes for POSIX ACLs
even when they are set.

This example on the listxattr manpage is rather unfortunate and should
be fixed; how filesystems implement stuff internally should really be
of no relevance here. I'm taking Michael into the CC.

> If this patch simplified the code, I'd be all for it;
> but it's the reverse, and we seem to have survived for several
> years without it: I don't see yet why it's needed.

It's a confusing inconsistency. Many utilities like ls and
getfacl/setfacl check for the presence of POSIX ACLs with getxattr,
they don't use listxattr for that. Other utilities that rely on
listxattr currently won't see when objects on tmpfs have POSIX ACLs
though; this is bad.

> I've no
> fundamental objection, but I'd like to understand why it's
> a step forwards rather than a step backwards.

Yes, I hope I could explain that well enough.

Thanks,
Andreas

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>