On 8/15/22 12:00 PM, Christian Brauner wrote:
> !-------------------------------------------------------------------|
> This Message Is From an External Sender
>
> |-------------------------------------------------------------------!
>
> On Mon, Aug 15, 2022 at 12:32:25PM +0800, kernel test robot wrote:
>>
>>
>> Greeting,
>>
>> FYI, we noticed a -9.0% regression of will-it-scale.per_thread_ops due to commit:
>>
>>
>> commit: faf99b563558f74188b7ca34faae1c1da49a7261 ("fs: add __remove_file_privs() with flags parameter")
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
>
> This seems overall pretty odd tbh at least it's not immediately obvious
> how that specific commit would've caused this. But fwiw, I think there's
> one issue in this change which we originally overlooked which might
> explain this.
>
> Before faf99b563558 ("fs: add __remove_file_privs() with flags
> parameter") inode_has_no_xattr() was called when
> dentry_needs_remove_privs() returned 0.
>
> int error = 0
> [...]
> kill = dentry_needs_remove_privs(dentry);
> if (kill < 0)
> return kill;
> if (kill)
> error = __remove_privs(file_mnt_user_ns(file), dentry, kill);
> if (!error)
> inode_has_no_xattr(inode);
>
> but now we do:
>
> kill = dentry_needs_remove_privs(dentry);
> if (kill <= 0)
> return kill;
>
> which means we don't call inode_has_no_xattr(). I don't think that we
> did this intentionally. inode_has_no_xattr() just sets S_NOSEC which
> means next time we call into __file_remove_privs() we can return earlier
> instead of hitting dentry_needs_remove_privs() again:
>
> if (IS_NOSEC(inode) || !S_ISREG(inode->i_mode))
> return 0;
>
> So I think that needs to be fixed?
>
Christian,
thanks for looking into this. I'll prepare a fix to maintain the original behavior.
--Stefan
> Christian
