Re: [kernel.org bug 103071] Dead "security.*" xattr code in ubifs

[Sheng Yong <shengyong1@huawei.com>]

On 8/19/2015 2:59 PM, Richard Weinberger wrote:
> Am 19.08.2015 um 03:37 schrieb Sheng Yong:
>>
>>
>> On 8/19/2015 9:07 AM, Sheng Yong wrote:
>>> Hi,
>>>
>>> On 8/19/2015 4:55 AM, Richard Weinberger wrote:
>>>> Andreas,
>>>>
>>>> On Tue, Aug 18, 2015 at 2:15 PM, Andreas Grünbacher
>>>> <andreas.gruenbacher@gmail.com> wrote:
>>>>> Hello,
>>>>>
>>>>> FYI, I've filed the following bug report against ubifs:
>>>>>
>>>>>> ubifs sets sb->s_xattr to ubifs_xattr_handlers which contains a handler for
>>>>>> "security.*" xattrs. The s_xattr handlers are never used because ubifs uses
>>>>>> its own ubifs_{get,set,list,remove}xattr inode operations instead of
>>>>>> generic_{get,set,list,remove}xattr inode operations though.
>>>>>
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=103071
>>>>
>>>> Thanks for reporting.
>> My bad for didn't reply at the right line :(
>>> This is because UBIFS did not implement extended attribute in the geneirc way
>>> (by calling generic_*xattr()). We could create an xattr_handler to have these
>>> dead functions called (in fact, I did that), but doing this seems just another
>>> encapsulation and makes no sense. So I think we could remove these dead code
>>> directly.
> 
> So, that would be a revert of commit d7f0b70d30ffb9bbe6b8a3e1035cf0b79965ef53?
I think we still need security xattr, so security stuff should be initialized
when creating inodes.
> I don't get what d7f0b70d30ffb9bbe6b8a3e1035cf0b79965ef53 was supposed to do
> as it seems to be a no-op. :-)
AFAIK, geneirc_*attr() will go though sb->s_xattr to find the xattr_handler
which matches the xattr prefix, and generic_*attr() should have been triggered
from inode_operations. However, UBIFS uses ubifs_*attr in inode_operations
instead of these generic interfaces. So `ubifs_xattr_security_handler' defined
in fs/ubifs/xattr.c is useless, so are the security_*attr() functions. These
functions will never being called, set/get/list security xattr is still done
by calling ubifs_*attr() like other xattr.

thanks,
Sheng
> 
> Thanks,
> //richard
> 
> .
>