[PATCH v1] fs: __file_remove_privs(): restore call to inode_has_no_xattr()

[PATCH v1] fs: __file_remove_privs(): restore call to inode_has_no_xattr()

[Stefan Roesch]

This restores the call to inode_has_no_xattr() in the function
__file_remove_privs(). In case the dentry_meeds_remove_privs() returned
0, the function inode_has_no_xattr() was not called.

Signed-off-by: Stefan Roesch <shr@fb.com>
---
 fs/inode.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 6462276dfdf0..ba1de23c13c1 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2018,23 +2018,25 @@ static int __file_remove_privs(struct file *file, unsigned int flags)
 {
 	struct dentry *dentry = file_dentry(file);
 	struct inode *inode = file_inode(file);
-	int error;
+	int error = 0;
 	int kill;
 
 	if (IS_NOSEC(inode) || !S_ISREG(inode->i_mode))
 		return 0;
 
 	kill = dentry_needs_remove_privs(dentry);
-	if (kill <= 0)
+	if (kill < 0)
 		return kill;
 
-	if (flags & IOCB_NOWAIT)
-		return -EAGAIN;
+	if (kill) {
+		if (flags & IOCB_NOWAIT)
+			return -EAGAIN;
+
+		error = __remove_privs(file_mnt_user_ns(file), dentry, kill);
+	}
 
-	error = __remove_privs(file_mnt_user_ns(file), dentry, kill);
 	if (!error)
 		inode_has_no_xattr(inode);
-
 	return error;
 }
 
-- 
2.30.2

Re: [PATCH v1] fs: __file_remove_privs(): restore call to inode_has_no_xattr()

[Jens Axboe]

On 8/16/22 9:31 AM, Stefan Roesch wrote:
> This restores the call to inode_has_no_xattr() in the function
> __file_remove_privs(). In case the dentry_meeds_remove_privs() returned
> 0, the function inode_has_no_xattr() was not called.

Should add a:

Fixes: faf99b563558 ("fs: add __remove_file_privs() with flags parameter")

to this commit message.

-- 
Jens Axboe

Re: [PATCH v1] fs: __file_remove_privs(): restore call to inode_has_no_xattr()

[Christian Brauner]

On Tue, Aug 16, 2022 at 08:31:58AM -0700, Stefan Roesch wrote:
> This restores the call to inode_has_no_xattr() in the function
> __file_remove_privs(). In case the dentry_meeds_remove_privs() returned
> 0, the function inode_has_no_xattr() was not called.
> 
> Signed-off-by: Stefan Roesch <shr@fb.com>
> ---

Looks good to me. As Jens said, this should get a fixes tag and should
probably also have a link to the perf regression I debugged this on so
that we can see whether this was actually the cause,
Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>

Re: [PATCH v1] fs: __file_remove_privs(): restore call to inode_has_no_xattr()

[Jens Axboe]

On 8/16/22 11:01 AM, Christian Brauner wrote:
> On Tue, Aug 16, 2022 at 08:31:58AM -0700, Stefan Roesch wrote:
>> This restores the call to inode_has_no_xattr() in the function
>> __file_remove_privs(). In case the dentry_meeds_remove_privs() returned
>> 0, the function inode_has_no_xattr() was not called.
>>
>> Signed-off-by: Stefan Roesch <shr@fb.com>
>> ---
> 
> Looks good to me. As Jens said, this should get a fixes tag and should
> probably also have a link to the perf regression I debugged this on so
> that we can see whether this was actually the cause,
> Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>

Yes good point, this is a case where the Link actually makes a lot of
sense.

-- 
Jens Axboe