lkml.org 
[lkml]   [2023]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] fs/{posix_acl,ext2,jfs,ceph}: apply umask if ACL support is disabled
    On Mon 09-10-23 16:43:39, Max Kellermann wrote:
    > One important implementation detail of the posix_acl_create() function
    > is that it applies the umask to the "mode" parameter. If
    > CONFIG_FS_POSIX_ACL is disabled, this detail is missing and the umask
    > may not get applied.
    >
    > This patch adds the missing code to posix_acl_create() and to three
    > filesystems that omit the posix_acl_create() call if their individual
    > ACL support is disabled (CONFIG_EXT2_FS_POSIX_ACL,
    > CONFIG_JFS_POSIX_ACL, CONFIG_CEPH_FS_POSIX_ACL). If
    > posix_acl_create() never gets called, the umask needs to be applied
    > anyway.
    >
    > This bug used to be exploitable easily with O_TMPFILE (see
    > https://bugzilla.kernel.org/show_bug.cgi?id=203625) but that part was
    > fixed by commit ac6800e279a2 ("fs: Add missing umask strip in
    > vfs_tmpfile") last year. The bug may not be reachable by userspace
    > anymore, but since it is apparently still necessary to apply the umask
    > again in posix_acl_create(), there is no reason to assume it's not
    > necessary with ACL support is disabled.
    >
    > Signed-off-by: Max Kellermann <max.kellermann@ionos.com>

    Thanks for the updated changelog! But as I'm looking into VFS code isn't
    this already handled by mode_strip_umask() / vfs_prepare_mode() in
    fs/namei.c? Because posix_acl_create() doesn't do anything to 'mode' for
    !IS_POSIXACL() filesystems either. So at least ext2 (where I've checked
    the mount option handling) does seem to have umask properly applied in all
    the cases. But I might be missing something...

    Honza

    > ---
    > fs/ceph/super.h | 6 ++++++
    > fs/ext2/acl.h | 6 ++++++
    > fs/jfs/jfs_acl.h | 6 ++++++
    > include/linux/posix_acl.h | 1 +
    > 4 files changed, 19 insertions(+)
    >
    > diff --git a/fs/ceph/super.h b/fs/ceph/super.h
    > index 51c7f2b14f6f..58349639bd57 100644
    > --- a/fs/ceph/super.h
    > +++ b/fs/ceph/super.h
    > @@ -1194,6 +1194,12 @@ static inline void ceph_forget_all_cached_acls(struct inode *inode)
    > static inline int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
    > struct ceph_acl_sec_ctx *as_ctx)
    > {
    > + /* usually, the umask is applied by posix_acl_create(), but if
    > + * ACL support is disabled at compile time, we need to do it
    > + * here, because posix_acl_create() will never be called
    > + */
    > + *mode &= ~current_umask();
    > +
    > return 0;
    > }
    > static inline void ceph_init_inode_acls(struct inode *inode,
    > diff --git a/fs/ext2/acl.h b/fs/ext2/acl.h
    > index 4a8443a2b8ec..0ecaa9c20c0c 100644
    > --- a/fs/ext2/acl.h
    > +++ b/fs/ext2/acl.h
    > @@ -67,6 +67,12 @@ extern int ext2_init_acl (struct inode *, struct inode *);
    >
    > static inline int ext2_init_acl (struct inode *inode, struct inode *dir)
    > {
    > + /* usually, the umask is applied by posix_acl_create(), but if
    > + * ACL support is disabled at compile time, we need to do it
    > + * here, because posix_acl_create() will never be called
    > + */
    > + inode->i_mode &= ~current_umask();
    > +
    > return 0;
    > }
    > #endif
    > diff --git a/fs/jfs/jfs_acl.h b/fs/jfs/jfs_acl.h
    > index f892e54d0fcd..64a05e663a45 100644
    > --- a/fs/jfs/jfs_acl.h
    > +++ b/fs/jfs/jfs_acl.h
    > @@ -17,6 +17,12 @@ int jfs_init_acl(tid_t, struct inode *, struct inode *);
    > static inline int jfs_init_acl(tid_t tid, struct inode *inode,
    > struct inode *dir)
    > {
    > + /* usually, the umask is applied by posix_acl_create(), but if
    > + * ACL support is disabled at compile time, we need to do it
    > + * here, because posix_acl_create() will never be called
    > + */
    > + inode->i_mode &= ~current_umask();
    > +
    > return 0;
    > }
    >
    > diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
    > index 0e65b3d634d9..54bc9b1061ca 100644
    > --- a/include/linux/posix_acl.h
    > +++ b/include/linux/posix_acl.h
    > @@ -128,6 +128,7 @@ static inline void cache_no_acl(struct inode *inode)
    > static inline int posix_acl_create(struct inode *inode, umode_t *mode,
    > struct posix_acl **default_acl, struct posix_acl **acl)
    > {
    > + *mode &= ~current_umask();
    > *default_acl = *acl = NULL;
    > return 0;
    > }
    > --
    > 2.39.2
    >
    --
    Jan Kara <jack@suse.com>
    SUSE Labs, CR

    \
     
     \ /
      Last update: 2023-10-10 15:12    [W:2.945 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site