Re: [PATCH] xattr handlers: fixup generic_listxattr

[Andreas Gruenbacher <agruenba@redhat.com>]

On Tue, May 17, 2016 at 3:12 AM, James Simmons <jsimmons@infradead.org> wrote:
>> generic_listxattr() is different from generic_getxattr() /
>> generic_setxattr() / generic_removexattr. It makes sense only for
>> filesystems that support a fixed set of xattrs, which means that all
>> handlers will have handler->name set.
>>
>> If any of the handlers has handler->prefix set instead, that handler
>> matches a whole set of attributes. Generic_listxattr() would have to
>> fill in all of those names matching that handler, but it doesn't know
>> which those are.
>>
>> It is common for filesystems to have their own listxattr inode
>> operation and still use generic_{get,set,remove}xattr.
>
> That clears things up a bit. So that leaves a few questions. First
> question is looking at several of the file system's implementations
> I noticed it contains loops such as:
>
> list_for_each_xattr(entry, base_addr) {
>         const struct xattr_handler *handler =
>                 blah_xattr_handler(entry->e_name_index);
>         const char *prefix;
>         size_t prefix_len;
>         size_t size;
>
>         if (!handler || (handler->list && !handler->list(dentry)))
>                 continue;
>
>         ...
> }
>
> Is the handler->list() test needed for a private listxattr implementation?
> Also I don't see anyone using handler->list() which which brings up the
> next question. What is the purpose of list() function?

Trusted xattrs are meant to only be visible to tasks with the
CAP_SYS_ADMIN capability. For deciding which names to include in its
result, listxattr implementations can use the list function, or any
other suitable mechanism. On filesystems that store the complete xattr
names as strings, a strncmp check for a "trusted." prefix together
with capable(CAP_SYS_ADMIN) would do, for example.

Andreas