Re: [PATCH v3 04/15] mm/shmem: honour F_SEAL_EXEC when an ACL is set

[Balbir Singh <balbirs@nvidia.com>]

On Wed, Oct 28, 2026 at 10:55:02AM +0100, David Hildenbrand (Arm) wrote:
> From: Chris Mason <mason@kernel.org>
> 
> The owner of a memfd sealed with F_SEAL_EXEC can still change its
> execute bits by setting a POSIX access ACL. With CONFIG_TMPFS_POSIX_ACL,
> this lets an unprivileged owner make a non-executable memfd executable,
> bypassing vm.memfd_noexec=2.
> 
> simple_set_acl() updates i_mode directly, bypassing the seal check in
> shmem_setattr(). Add shmem_set_acl() to check the resulting mode. Share
> the F_SEAL_EXEC check with shmem_setattr() using memfd_check_seals_mode()
> in include/linux/memfd.h.
> 
> Fix generated by an LLM and sent by Chris Mason in private.
> 
> Fixes: 6fd7353829ca ("mm/memfd: add F_SEAL_EXEC")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chris Mason <mason@kernel.org>
> Acked-by: Pedro Falcato <pfalcato@suse.de>
> Acked-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> Acked-by: Zi Yan <ziy@nvidia.com>
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> Reviewed-by: Pratyush Yadav (Google) <pratyush@kernel.org>
> Tested-by: Gregory Price (Meta) <gourry@gourry.net>
> [ david: use common helper; simplify description ]
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> ---
>  include/linux/memfd.h |  9 +++++++++
>  mm/shmem.c            | 35 ++++++++++++++++++++++++++++++-----
>  2 files changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/memfd.h b/include/linux/memfd.h
> index c159e40e3f34..272479317cf1 100644
> --- a/include/linux/memfd.h
> +++ b/include/linux/memfd.h
> @@ -2,10 +2,19 @@
>  #ifndef __LINUX_MEMFD_H
>  #define __LINUX_MEMFD_H
>  
> +#include <linux/fcntl.h>
>  #include <linux/file.h>
>  
>  #define MEMFD_ANON_NAME "[memfd]"
>  
> +static inline int memfd_check_seals_mode(unsigned int seals, umode_t old_mode,
> +		umode_t new_mode)
> +{
> +	if ((seals & F_SEAL_EXEC) && ((old_mode ^ new_mode) & 0111))
> +		return -EPERM;
> +	return 0;
> +}
> +
>  #ifdef CONFIG_MEMFD_CREATE
>  extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned int arg);
>  struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx);
> diff --git a/mm/shmem.c b/mm/shmem.c
> index af21a89abb0f..e3e4bbbe1d80 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -77,6 +77,7 @@ static struct vfsmount *shm_mnt __ro_after_init;
>  #include <linux/magic.h>
>  #include <linux/syscalls.h>
>  #include <linux/fcntl.h>
> +#include <linux/memfd.h>
>  #include <uapi/linux/memfd.h>
>  #include <linux/rmap.h>
>  #include <linux/uuid.h>
> @@ -1546,10 +1547,11 @@ static int shmem_setattr(const struct mnt_idmap *idmap,
>  	if (error)
>  		return error;
>  
> -	if ((info->seals & F_SEAL_EXEC) && (attr->ia_valid & ATTR_MODE)) {
> -		if ((inode->i_mode ^ attr->ia_mode) & 0111) {
> -			return -EPERM;
> -		}
> +	if (attr->ia_valid & ATTR_MODE) {
> +		error = memfd_check_seals_mode(info->seals, inode->i_mode,
> +					       attr->ia_mode);
> +		if (error)
> +			return error;
>  	}
>  
>  	if (S_ISREG(inode->i_mode) && (attr->ia_valid & ATTR_SIZE)) {
> @@ -5404,12 +5406,35 @@ static const struct file_operations shmem_file_operations = {
>  #endif
>  };
>  
> +#ifdef CONFIG_TMPFS_POSIX_ACL
> +static int shmem_set_acl(const struct mnt_idmap *idmap, struct dentry *dentry,
> +		struct posix_acl *acl, int type)
> +{
> +	struct inode *inode = d_inode(dentry);
> +	umode_t mode = inode->i_mode;
> +	int error;
> +
> +	if (type == ACL_TYPE_ACCESS) {
> +		error = posix_acl_equiv_mode(acl, &mode);
> +		if (error < 0)
> +			return error;
> +		error = memfd_check_seals_mode(SHMEM_I(inode)->seals,
> +					       inode->i_mode, mode);
> +		if (error)
> +			return error;
> +	}
> +	return simple_set_acl(idmap, dentry, acl, type);
> +}
> +#else
> +#define shmem_set_acl NULL
> +#endif
> +
>  static const struct inode_operations shmem_inode_operations = {
>  	.getattr	= shmem_getattr,
>  	.setattr	= shmem_setattr,
>  #ifdef CONFIG_TMPFS_XATTR
>  	.listxattr	= shmem_listxattr,
> -	.set_acl	= simple_set_acl,
> +	.set_acl	= shmem_set_acl,

If CONFIG_TMPFS_POSIX_ACL is not defined, do we want to set the set_acl
callback to NULL? Earlier it was simple_set_acl

>  	.fileattr_get	= shmem_fileattr_get,
>  	.fileattr_set	= shmem_fileattr_set,
>  #endif
> 
> -- 
> 2.43.0
> 
>