diff mbox series

[v5,2/4] fs: Add missing umask strip in vfs_tmpfile

Message ID 1650527658-2218-2-git-send-email-xuyang2018.jy@fujitsu.com (mailing list archive)
State New, archived
Headers show
Series [v5,1/4] fs: move sgid strip operation from inode_init_owner into inode_sgid_strip | expand

Commit Message

Yang Xu (Fujitsu) April 21, 2022, 7:54 a.m. UTC 
All creation paths except for O_TMPFILE handle umask in the vfs directly
if the filesystem doesn't support or enable POSIX ACLs. If the filesystem
does then umask handling is deferred until posix_acl_create().
Because, O_TMPFILE misses umask handling in the vfs it will not honor
umask settings. Fix this by adding the missing umask handling.

Reported-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Acked-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 fs/namei.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Christian Brauner April 21, 2022, 7:24 a.m. UTC | #1 
On Thu, Apr 21, 2022 at 03:54:16PM +0800, Yang Xu wrote:
> All creation paths except for O_TMPFILE handle umask in the vfs directly
> if the filesystem doesn't support or enable POSIX ACLs. If the filesystem
> does then umask handling is deferred until posix_acl_create().
> Because, O_TMPFILE misses umask handling in the vfs it will not honor
> umask settings. Fix this by adding the missing umask handling.
> 
> Reported-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> Acked-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---

So given that we seem to all agree that the missing umask stripping is a
bug and not intentional because of special O_TMPFILE semantics (which
wouldn't have suprised me tbh...) this should get a:

Fixes: 60545d0d4610 ("[O_TMPFILE] it's still short a few helpers, but infrastructure should be OK now...")
Cc: <stable@vger.kernel.org> # 4.19+

If people feel comfortable it'd be great to get some more acks on this
or an explanation why umask doesn't need to be stripped in this case...
diff mbox series

Patch

diff --git a/fs/namei.c b/fs/namei.c
index 509657fdf4f5..73646e28fae0 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3521,6 +3521,8 @@  struct dentry *vfs_tmpfile(struct user_namespace *mnt_userns,
 	child = d_alloc(dentry, &slash_name);
 	if (unlikely(!child))
 		goto out_err;
+	if (!IS_POSIXACL(dir))
+		mode &= ~current_umask();
 	error = dir->i_op->tmpfile(mnt_userns, dir, child, mode);
 	if (error)
 		goto out_err;