| Message ID | 20220816153158.1925040-1-shr@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] fs: __file_remove_privs(): restore call to inode_has_no_xattr() | expand |
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.
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>
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.
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; }
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(-)