Skip to contentSkip to conversation

mm: do not grant +x by default in memfd_create()

PATCH Superseded Dima Krasner 2 participants 3 messages May 4, 2019 – May 5, 2019

Subsystems (4)
Memory management Memory mapping HugeTLB tmpfs (shmem filesystem)
lore
Share & tools
lore

Dima Krasner

author
#0 lore raw

PATCH 1/1 mm: do not grant +x by default in memfd_create()

This syscall allows easy fileless execution, without calling chmod() first. Thus, some security-related restrictions (like seccomp filters that deny chmod +x) can by bypassed using memfd_create() if the policy author is unaware of this.

Signed-off-by Dima Krasner <[email protected]>
Cc [email protected]
Cc Hugh Dickins <[email protected]>
---
 fs/hugetlbfs/inode.c    |  5 +++--
 include/linux/hugetlb.h |  2 +-
 include/linux/stat.h    |  1 +
 ipc/shm.c               |  3 ++-
 mm/memfd.c              |  3 ++-
 mm/mmap.c               |  3 ++-
 mm/shmem.c              | 11 ++++++-----
 7 files changed, 17 insertions(+), 11 deletions(-)
fs/hugetlbfs/inode.c +3 -2
index 9285dd4f4b1c..512ac8a226ef 100644
@@ -1363,7 +1363,8 @@ static int get_hstate_idx(int page_size_log)
  */
 struct file *hugetlb_file_setup(const char *name, size_t size,
 				vm_flags_t acctflag, struct user_struct **user,
-				int creat_flags, int page_size_log)
+				int creat_flags, int page_size_log,
+				umode_t mode)
 {
 	struct inode *inode;
 	struct vfsmount *mnt;

@@ -1393,7 +1394,7 @@ struct file *hugetlb_file_setup(const char *name, size_t size,
 	}
 
 	file = ERR_PTR(-ENOSPC);
-	inode = hugetlbfs_get_inode(mnt->mnt_sb, NULL, S_IFREG | S_IRWXUGO, 0);
+	inode = hugetlbfs_get_inode(mnt->mnt_sb, NULL, S_IFREG | mode, 0);
 	if (!inode)
 		goto out;
 	if (creat_flags == HUGETLB_SHMFS_INODE)
include/linux/hugetlb.h +1 -1
index 11943b60f208..ae17f6629e80 100644
@@ -308,7 +308,7 @@ extern const struct file_operations hugetlbfs_file_operations;
 extern const struct vm_operations_struct hugetlb_vm_ops;
 struct file *hugetlb_file_setup(const char *name, size_t size, vm_flags_t acct,
 				struct user_struct **user, int creat_flags,
-				int page_size_log);
+				int page_size_log, umode_t mode);
 
 static inline bool is_file_hugepages(struct file *file)
 {
include/linux/stat.h +1 -0
index 765573dc17d6..efda7df06d6e 100644
@@ -11,6 +11,7 @@
 #define S_IRUGO		(S_IRUSR|S_IRGRP|S_IROTH)
 #define S_IWUGO		(S_IWUSR|S_IWGRP|S_IWOTH)
 #define S_IXUGO		(S_IXUSR|S_IXGRP|S_IXOTH)
+#define S_IRWUGO	(S_IRUGO|S_IWUGO)
 
 #define UTIME_NOW	((1l << 30) - 1l)
 #define UTIME_OMIT	((1l << 30) - 2l)
ipc/shm.c +2 -1
index ce1ca9f7c6e9..a64a52e4ccf1 100644
@@ -651,7 +651,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 			acctflag = VM_NORESERVE;
 		file = hugetlb_file_setup(name, hugesize, acctflag,
 				  &shp->mlock_user, HUGETLB_SHMFS_INODE,
-				(shmflg >> SHM_HUGE_SHIFT) & SHM_HUGE_MASK);
+				(shmflg >> SHM_HUGE_SHIFT) & SHM_HUGE_MASK,
+				S_IRWXUGO);
 	} else {
 		/*
 		 * Do not allow no accounting for OVERCOMMIT_NEVER, even
mm/memfd.c +2 -1
index 650e65a46b9c..b680fae87240 100644
@@ -300,7 +300,8 @@ SYSCALL_DEFINE2(memfd_create,
 		file = hugetlb_file_setup(name, 0, VM_NORESERVE, &user,
 					HUGETLB_ANONHUGE_INODE,
 					(flags >> MFD_HUGE_SHIFT) &
-					MFD_HUGE_MASK);
+					MFD_HUGE_MASK,
+					S_IRWUGO);
 	} else
 		file = shmem_file_setup(name, 0, VM_NORESERVE);
 	if (IS_ERR(file)) {
mm/mmap.c +2 -1
index bd7b9f293b39..4494ce44ca5d 100644
@@ -1600,7 +1600,8 @@ unsigned long ksys_mmap_pgoff(unsigned long addr, unsigned long len,
 		file = hugetlb_file_setup(HUGETLB_ANON_FILE, len,
 				VM_NORESERVE,
 				&user, HUGETLB_ANONHUGE_INODE,
-				(flags >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK);
+				(flags >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK,
+				S_IRWXUGO);
 		if (IS_ERR(file))
 			return PTR_ERR(file);
 	}
mm/shmem.c +6 -5
index 2275a0ff7c30..a903b9f783e2 100644
@@ -3968,7 +3968,8 @@ EXPORT_SYMBOL_GPL(shmem_truncate_range);
 /* common code */
 
 static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, loff_t size,
-				       unsigned long flags, unsigned int i_flags)
+				       unsigned long flags, unsigned int i_flags,
+				       umode_t mode)
 {
 	struct inode *inode;
 	struct file *res;

@@ -3982,7 +3983,7 @@ static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, l
 	if (shmem_acct_size(flags, size))
 		return ERR_PTR(-ENOMEM);
 
-	inode = shmem_get_inode(mnt->mnt_sb, NULL, S_IFREG | S_IRWXUGO, 0,
+	inode = shmem_get_inode(mnt->mnt_sb, NULL, S_IFREG | mode, 0,
 				flags);
 	if (unlikely(!inode)) {
 		shmem_unacct_size(flags, size);

@@ -4012,7 +4013,7 @@ static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, l
  */
 struct file *shmem_kernel_file_setup(const char *name, loff_t size, unsigned long flags)
 {
-	return __shmem_file_setup(shm_mnt, name, size, flags, S_PRIVATE);
+	return __shmem_file_setup(shm_mnt, name, size, flags, S_PRIVATE, S_IRWXUGO);
 }
 
 /**

@@ -4023,7 +4024,7 @@ struct file *shmem_kernel_file_setup(const char *name, loff_t size, unsigned lon
  */
 struct file *shmem_file_setup(const char *name, loff_t size, unsigned long flags)
 {
-	return __shmem_file_setup(shm_mnt, name, size, flags, 0);
+	return __shmem_file_setup(shm_mnt, name, size, flags, 0, S_IRWUGO);
 }
 EXPORT_SYMBOL_GPL(shmem_file_setup);
 

@@ -4037,7 +4038,7 @@ EXPORT_SYMBOL_GPL(shmem_file_setup);
 struct file *shmem_file_setup_with_mnt(struct vfsmount *mnt, const char *name,
 				       loff_t size, unsigned long flags)
 {
-	return __shmem_file_setup(mnt, name, size, flags, 0);
+	return __shmem_file_setup(mnt, name, size, flags, 0, S_IRWUGO);
 }
 EXPORT_SYMBOL_GPL(shmem_file_setup_with_mnt);
Signature · 1 line
2.21.0
build bot
#1 lore raw
Thank you for the patch! Yet something to improve:

Hi Dima,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.1-rc7 next-20190503]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Dima-Krasner/mm-do-not-grant-x-by-default-in-memfd_create/20190505-060301
config: i386-randconfig-x070-201918 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce:

        # save the attached .config to linux build tree
        make ARCH=i386

If you fix the issue, kindly add following tag

Reported-by kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

mm/memfd.c: In function '__do_sys_memfd_create':

>> mm/memfd.c:300:10: error: too many arguments to function 'hugetlb_file_setup'

file = hugetlb_file_setup(name, 0, VM_NORESERVE, &user,
^~~~~~~~~~~~~~~~~~
In file included from mm/memfd.c:18:0:
include/linux/hugetlb.h:326:1: note: declared here
hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag, ^~~~~~~~~~~~~~~~~~
--
ipc/shm.c: In function 'newseg':

>> ipc/shm.c:652:10: error: too many arguments to function 'hugetlb_file_setup'

file = hugetlb_file_setup(name, hugesize, acctflag,
^~~~~~~~~~~~~~~~~~
In file included from ipc/shm.c:30:0:
include/linux/hugetlb.h:326:1: note: declared here
hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag, ^~~~~~~~~~~~~~~~~~

vim +/hugetlb_file_setup +300 mm/memfd.c

5d752600 Mike Kravetz 2018-06-07  247  
5d752600 Mike Kravetz 2018-06-07  248  SYSCALL_DEFINE2(memfd_create,
5d752600 Mike Kravetz 2018-06-07  249  		const char __user *, uname,
5d752600 Mike Kravetz 2018-06-07  250  		unsigned int, flags)
5d752600 Mike Kravetz 2018-06-07  251  {
5d752600 Mike Kravetz 2018-06-07  252  	unsigned int *file_seals;
5d752600 Mike Kravetz 2018-06-07  253  	struct file *file;
5d752600 Mike Kravetz 2018-06-07  254  	int fd, error;
5d752600 Mike Kravetz 2018-06-07  255  	char *name;
5d752600 Mike Kravetz 2018-06-07  256  	long len;
5d752600 Mike Kravetz 2018-06-07  257  
5d752600 Mike Kravetz 2018-06-07  258  	if (!(flags & MFD_HUGETLB)) {
5d752600 Mike Kravetz 2018-06-07  259  		if (flags & ~(unsigned int)MFD_ALL_FLAGS)
5d752600 Mike Kravetz 2018-06-07  260  			return -EINVAL;
5d752600 Mike Kravetz 2018-06-07  261  	} else {
5d752600 Mike Kravetz 2018-06-07  262  		/* Allow huge page size encoding in flags. */
5d752600 Mike Kravetz 2018-06-07  263  		if (flags & ~(unsigned int)(MFD_ALL_FLAGS |
5d752600 Mike Kravetz 2018-06-07  264  				(MFD_HUGE_MASK << MFD_HUGE_SHIFT)))
5d752600 Mike Kravetz 2018-06-07  265  			return -EINVAL;
5d752600 Mike Kravetz 2018-06-07  266  	}
5d752600 Mike Kravetz 2018-06-07  267  
5d752600 Mike Kravetz 2018-06-07  268  	/* length includes terminating zero */
5d752600 Mike Kravetz 2018-06-07  269  	len = strnlen_user(uname, MFD_NAME_MAX_LEN + 1);
5d752600 Mike Kravetz 2018-06-07  270  	if (len <= 0)
5d752600 Mike Kravetz 2018-06-07  271  		return -EFAULT;
5d752600 Mike Kravetz 2018-06-07  272  	if (len > MFD_NAME_MAX_LEN + 1)
5d752600 Mike Kravetz 2018-06-07  273  		return -EINVAL;
5d752600 Mike Kravetz 2018-06-07  274  
5d752600 Mike Kravetz 2018-06-07  275  	name = kmalloc(len + MFD_NAME_PREFIX_LEN, GFP_KERNEL);
5d752600 Mike Kravetz 2018-06-07  276  	if (!name)
5d752600 Mike Kravetz 2018-06-07  277  		return -ENOMEM;
5d752600 Mike Kravetz 2018-06-07  278  
5d752600 Mike Kravetz 2018-06-07  279  	strcpy(name, MFD_NAME_PREFIX);
5d752600 Mike Kravetz 2018-06-07  280  	if (copy_from_user(&name[MFD_NAME_PREFIX_LEN], uname, len)) {
5d752600 Mike Kravetz 2018-06-07  281  		error = -EFAULT;
5d752600 Mike Kravetz 2018-06-07  282  		goto err_name;
5d752600 Mike Kravetz 2018-06-07  283  	}
5d752600 Mike Kravetz 2018-06-07  284  
5d752600 Mike Kravetz 2018-06-07  285  	/* terminating-zero may have changed after strnlen_user() returned */
5d752600 Mike Kravetz 2018-06-07  286  	if (name[len + MFD_NAME_PREFIX_LEN - 1]) {
5d752600 Mike Kravetz 2018-06-07  287  		error = -EFAULT;
5d752600 Mike Kravetz 2018-06-07  288  		goto err_name;
5d752600 Mike Kravetz 2018-06-07  289  	}
5d752600 Mike Kravetz 2018-06-07  290  
5d752600 Mike Kravetz 2018-06-07  291  	fd = get_unused_fd_flags((flags & MFD_CLOEXEC) ? O_CLOEXEC : 0);
5d752600 Mike Kravetz 2018-06-07  292  	if (fd < 0) {
5d752600 Mike Kravetz 2018-06-07  293  		error = fd;
5d752600 Mike Kravetz 2018-06-07  294  		goto err_name;
5d752600 Mike Kravetz 2018-06-07  295  	}
5d752600 Mike Kravetz 2018-06-07  296  
5d752600 Mike Kravetz 2018-06-07  297  	if (flags & MFD_HUGETLB) {
5d752600 Mike Kravetz 2018-06-07  298  		struct user_struct *user = NULL;
5d752600 Mike Kravetz 2018-06-07  299

5d752600 Mike Kravetz 2018-06-07 @300 file = hugetlb_file_setup(name, 0, VM_NORESERVE, &user,

:::::: The code at line 300 was first introduced by commit :::::: 5d752600a8c373382264392f5b573b2fc9c0e8ea mm: restructure memfd code

:::::: TO: Mike Kravetz <[email protected]> :::::: CC: Linus Torvalds <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

author
#2 lore raw

PATCH v2 1/1 mm: do not grant +x by default in memfd_create()

This syscall allows easy fileless execution, without calling chmod() first. Thus, some security-related restrictions (like seccomp filters that deny chmod +x) can by bypassed using memfd_create() if the policy author is unaware of this.

Signed-off-by Dima Krasner <[email protected]>
Cc [email protected]
Cc Hugh Dickins <[email protected]>
---
 fs/hugetlbfs/inode.c    |  5 +++--
 include/linux/hugetlb.h |  4 ++--
 include/linux/stat.h    |  1 +
 ipc/shm.c               |  3 ++-
 mm/memfd.c              |  3 ++-
 mm/mmap.c               |  3 ++-
 mm/shmem.c              | 11 ++++++-----
 7 files changed, 18 insertions(+), 12 deletions(-)
fs/hugetlbfs/inode.c +3 -2
index 9285dd4f4b1c..512ac8a226ef 100644
@@ -1363,7 +1363,8 @@ static int get_hstate_idx(int page_size_log)
  */
 struct file *hugetlb_file_setup(const char *name, size_t size,
 				vm_flags_t acctflag, struct user_struct **user,
-				int creat_flags, int page_size_log)
+				int creat_flags, int page_size_log,
+				umode_t mode)
 {
 	struct inode *inode;
 	struct vfsmount *mnt;

@@ -1393,7 +1394,7 @@ struct file *hugetlb_file_setup(const char *name, size_t size,
 	}
 
 	file = ERR_PTR(-ENOSPC);
-	inode = hugetlbfs_get_inode(mnt->mnt_sb, NULL, S_IFREG | S_IRWXUGO, 0);
+	inode = hugetlbfs_get_inode(mnt->mnt_sb, NULL, S_IFREG | mode, 0);
 	if (!inode)
 		goto out;
 	if (creat_flags == HUGETLB_SHMFS_INODE)
include/linux/hugetlb.h +2 -2
index 11943b60f208..59486f5ea89f 100644
@@ -308,7 +308,7 @@ extern const struct file_operations hugetlbfs_file_operations;
 extern const struct vm_operations_struct hugetlb_vm_ops;
 struct file *hugetlb_file_setup(const char *name, size_t size, vm_flags_t acct,
 				struct user_struct **user, int creat_flags,
-				int page_size_log);
+				int page_size_log, umode_t mode);
 
 static inline bool is_file_hugepages(struct file *file)
 {

@@ -325,7 +325,7 @@ static inline bool is_file_hugepages(struct file *file)
 static inline struct file *
 hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag,
 		struct user_struct **user, int creat_flags,
-		int page_size_log)
+		int page_size_log, umode_t mode)
 {
 	return ERR_PTR(-ENOSYS);
 }
include/linux/stat.h +1 -0
index 765573dc17d6..efda7df06d6e 100644
@@ -11,6 +11,7 @@
 #define S_IRUGO		(S_IRUSR|S_IRGRP|S_IROTH)
 #define S_IWUGO		(S_IWUSR|S_IWGRP|S_IWOTH)
 #define S_IXUGO		(S_IXUSR|S_IXGRP|S_IXOTH)
+#define S_IRWUGO	(S_IRUGO|S_IWUGO)
 
 #define UTIME_NOW	((1l << 30) - 1l)
 #define UTIME_OMIT	((1l << 30) - 2l)
ipc/shm.c +2 -1
index ce1ca9f7c6e9..a64a52e4ccf1 100644
@@ -651,7 +651,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 			acctflag = VM_NORESERVE;
 		file = hugetlb_file_setup(name, hugesize, acctflag,
 				  &shp->mlock_user, HUGETLB_SHMFS_INODE,
-				(shmflg >> SHM_HUGE_SHIFT) & SHM_HUGE_MASK);
+				(shmflg >> SHM_HUGE_SHIFT) & SHM_HUGE_MASK,
+				S_IRWXUGO);
 	} else {
 		/*
 		 * Do not allow no accounting for OVERCOMMIT_NEVER, even
mm/memfd.c +2 -1
index 650e65a46b9c..b680fae87240 100644
@@ -300,7 +300,8 @@ SYSCALL_DEFINE2(memfd_create,
 		file = hugetlb_file_setup(name, 0, VM_NORESERVE, &user,
 					HUGETLB_ANONHUGE_INODE,
 					(flags >> MFD_HUGE_SHIFT) &
-					MFD_HUGE_MASK);
+					MFD_HUGE_MASK,
+					S_IRWUGO);
 	} else
 		file = shmem_file_setup(name, 0, VM_NORESERVE);
 	if (IS_ERR(file)) {
mm/mmap.c +2 -1
index bd7b9f293b39..4494ce44ca5d 100644
@@ -1600,7 +1600,8 @@ unsigned long ksys_mmap_pgoff(unsigned long addr, unsigned long len,
 		file = hugetlb_file_setup(HUGETLB_ANON_FILE, len,
 				VM_NORESERVE,
 				&user, HUGETLB_ANONHUGE_INODE,
-				(flags >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK);
+				(flags >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK,
+				S_IRWXUGO);
 		if (IS_ERR(file))
 			return PTR_ERR(file);
 	}
mm/shmem.c +6 -5
index 2275a0ff7c30..a903b9f783e2 100644
@@ -3968,7 +3968,8 @@ EXPORT_SYMBOL_GPL(shmem_truncate_range);
 /* common code */
 
 static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, loff_t size,
-				       unsigned long flags, unsigned int i_flags)
+				       unsigned long flags, unsigned int i_flags,
+				       umode_t mode)
 {
 	struct inode *inode;
 	struct file *res;

@@ -3982,7 +3983,7 @@ static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, l
 	if (shmem_acct_size(flags, size))
 		return ERR_PTR(-ENOMEM);
 
-	inode = shmem_get_inode(mnt->mnt_sb, NULL, S_IFREG | S_IRWXUGO, 0,
+	inode = shmem_get_inode(mnt->mnt_sb, NULL, S_IFREG | mode, 0,
 				flags);
 	if (unlikely(!inode)) {
 		shmem_unacct_size(flags, size);

@@ -4012,7 +4013,7 @@ static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, l
  */
 struct file *shmem_kernel_file_setup(const char *name, loff_t size, unsigned long flags)
 {
-	return __shmem_file_setup(shm_mnt, name, size, flags, S_PRIVATE);
+	return __shmem_file_setup(shm_mnt, name, size, flags, S_PRIVATE, S_IRWXUGO);
 }
 
 /**

@@ -4023,7 +4024,7 @@ struct file *shmem_kernel_file_setup(const char *name, loff_t size, unsigned lon
  */
 struct file *shmem_file_setup(const char *name, loff_t size, unsigned long flags)
 {
-	return __shmem_file_setup(shm_mnt, name, size, flags, 0);
+	return __shmem_file_setup(shm_mnt, name, size, flags, 0, S_IRWUGO);
 }
 EXPORT_SYMBOL_GPL(shmem_file_setup);
 

@@ -4037,7 +4038,7 @@ EXPORT_SYMBOL_GPL(shmem_file_setup);
 struct file *shmem_file_setup_with_mnt(struct vfsmount *mnt, const char *name,
 				       loff_t size, unsigned long flags)
 {
-	return __shmem_file_setup(mnt, name, size, flags, 0);
+	return __shmem_file_setup(mnt, name, size, flags, 0, S_IRWUGO);
 }
 EXPORT_SYMBOL_GPL(shmem_file_setup_with_mnt);
Signature · 1 line
2.21.0