Deny further open for writing to files mmapped for execution

Commit 3d01ead ("Allow mmap() for exec on owned and non reachable temp
files") allows files created with the O_TMPFILE and O_EXCL open flags to be
mmapped for execution without content check.

However, it wrongly assumed that nobody could write that file except for
its creator. A malicious program could perform a write with this simple
command:

echo content > /proc/<PID of victim>/fd/<fd of file opened with O_TMPFILE>

Let files created with O_TMPFILE and anonymous inodes residing in a tmpfs
filesystem mounted by the kernel (e.g. those created by memfd_create()) to
be mmapped for execution without content check.

For the former group require that no more than an open for writing is
performed, and for the latter group no open at all (memfd_create() does not
invoke the file_open LSM hook).

If the mmap for execution was granted without content check, deny any
further open for writing.

These conditions have been defined with the assumption that letting a
process to execute content that itself provides is safe.

Signed-off-by: Roberto Sassu <[email protected]>

ebpf/Makefile.am

@@ -12,8 +12,8 @@ DIGLIM_CFLAGS=-DHAVE_KERNEL_PATCHES
 endif
 
 diglim_kern.o: vmlinux.h diglim_kern.c
-	clang -g -O2 -I$(top_srcdir)/include -target bpf -c diglim_kern.c \
-	      -mcpu=probe $(DIGLIM_CFLAGS) -o diglim_kern.o
+	clang -g -Wall -Werror -O2 -I$(top_srcdir)/include -target bpf \
+	      -c diglim_kern.c -mcpu=probe $(DIGLIM_CFLAGS) -o diglim_kern.o
 
 diglim_kern.skel.h: diglim_kern.o
 	/usr/sbin/bpftool gen skeleton diglim_kern.o > diglim_kern.skel.h

ebpf/diglim_kern.c

@@ -38,6 +38,12 @@
 #define __O_TMPFILE	020000000
 #define O_EXCL		00000200
 
+/* From include/linux/fs.h. */
+#define SB_KERNMOUNT	(1<<22) /* this is a kern_mount call */
+
+/* From include/uapi/linux/magic.h. */
+#define TMPFS_MAGIC	0x01021994
+
 char _license[] SEC("license") = "GPL";
 int lsm_mode;
 
@@ -70,7 +76,6 @@ static void log(enum errors error, u8 *digest, struct file *file)
 	struct task_struct *task = (struct task_struct *)bpf_get_current_task();
 	struct dentry *file_dentry = BPF_CORE_READ(file, f_path.dentry);
 	struct log_entry *e;
-	int i;
 
 	e = bpf_ringbuf_reserve(&ringbuf, sizeof(*e), 0);
 	if (!e)
@@ -131,14 +136,26 @@ static int digest_lookup(struct file *file)
 static bool mmap_exec_allowed(struct file *file)
 {
 	struct inode_storage *inode_storage;
+	u64 storage_flags = 0;
+
+	if ((file->f_inode->i_sb->s_magic == TMPFS_MAGIC) &&
+	     (file->f_inode->i_sb->s_flags & SB_KERNMOUNT))
+		storage_flags = BPF_LOCAL_STORAGE_GET_F_CREATE;
 
 	inode_storage = bpf_inode_storage_get(&inode_storage_map, file->f_inode,
-					      0, 0);
-	if (inode_storage &&
-	    (inode_storage->state & INODE_STATE_MMAP_EXEC_ALLOWED))
-		return true;
+					      0, storage_flags);
+	if (!inode_storage)
+		return false;
+
+	if (storage_flags &
+	    !(inode_storage->state & INODE_STATE_OPENED_WRITTEN))
+		inode_storage->state |= INODE_STATE_MMAP_EXEC_ALLOWED;
 
-	return false;
+	if (!(inode_storage->state & INODE_STATE_MMAP_EXEC_ALLOWED))
+		return false;
+
+	inode_storage->state |= INODE_STATE_MMAP_EXEC_DONE;
+	return true;
 }
 
 SEC("lsm.s/bprm_creds_for_exec")
@@ -173,7 +190,10 @@ int BPF_PROG(file_mprotect, struct vm_area_struct *vma, unsigned long prot)
 	if (!vma->vm_file || !(prot & PROT_EXEC) || (vma->vm_flags & VM_EXEC))
 		return 0;
 
-	log(MPROTECT_ERR, NULL, vma->vm_file ?: NULL);
+	if (mmap_exec_allowed(vma->vm_file))
+		return 0;
+
+	log(MPROTECT_ERR, NULL, vma->vm_file);
 	return (lsm_mode == MODE_ENFORCING) ? -EPERM : 0;
 }
 
@@ -186,24 +206,30 @@ int BPF_PROG(file_open, struct file *file)
 	if (!(file->f_mode & FMODE_WRITE))
 		return 0;
 
-	if ((file->f_flags & __O_TMPFILE) && (file->f_flags & O_EXCL))
+	if ((file->f_flags & __O_TMPFILE) ||
+	    ((file->f_inode->i_sb->s_magic == TMPFS_MAGIC) &&
+	     (file->f_inode->i_sb->s_flags & SB_KERNMOUNT)))
 		storage_flags = BPF_LOCAL_STORAGE_GET_F_CREATE;
 
 	inode_storage = bpf_inode_storage_get(&inode_storage_map, file->f_inode,
-					0, storage_flags);
+					      0, storage_flags);
 	if (!inode_storage)
 		return 0;
 
-	/*
-	 * Temp file not reachable by any other process than the one that
-	 * created it.
-	 */
-	if (file->f_flags & __O_TMPFILE) {
-		inode_storage->state |= INODE_STATE_MMAP_EXEC_ALLOWED;
-		return 0;
+	if (inode_storage->state & INODE_STATE_MMAP_EXEC_DONE) {
+		log(WRITE_MMAPPED_EXEC, NULL, file);
+		return (lsm_mode == MODE_ENFORCING) ? -EPERM : 0;
+	}
+
+	if (inode_storage->state & INODE_STATE_OPENED_WRITTEN) {
+		inode_storage->state &= ~INODE_STATE_MMAP_EXEC_ALLOWED;
+	} else {
+		if (file->f_flags & __O_TMPFILE)
+			inode_storage->state |= INODE_STATE_MMAP_EXEC_ALLOWED;
 	}
 
 	inode_storage->state &= ~INODE_STATE_CHECKED;
+	inode_storage->state |= INODE_STATE_OPENED_WRITTEN;
 	return 0;
 }
 

ebpf/diglim_user.c

@@ -33,6 +33,7 @@ static char *errors_str[LAST__ERR] = {
 	[CALC_DIGEST_ERR] = CALC_DIGEST_ERR_STR,
 	[MMAP_WRITERS_ERR] = MMAP_WRITERS_ERR_STR,
 	[MPROTECT_ERR] = MPROTECT_ERR_STR,
+	[WRITE_MMAPPED_EXEC] = WRITE_MMAPPED_EXEC_STR,
 };
 
 static int process_log(void *ctx, void *data, size_t len)

include/common_kern.h

@@ -12,6 +12,8 @@
 /* Inode state flags. */
 #define INODE_STATE_CHECKED		0x01
 #define INODE_STATE_MMAP_EXEC_ALLOWED	0x02
+#define INODE_STATE_MMAP_EXEC_DONE	0x04
+#define INODE_STATE_OPENED_WRITTEN	0x08
 
 /* Inode attrib flags. */
 #define INODE_ATTRIB_IMMUTABLE		0x01

include/log.h

@@ -11,12 +11,13 @@
 #define CALC_DIGEST_ERR_STR "cannot calculate the digest"
 #define MMAP_WRITERS_ERR_STR "mmap() with writers"
 #define MPROTECT_ERR_STR "mprotect() with exec perm"
+#define WRITE_MMAPPED_EXEC_STR "attempt to write a file mmapped for execution"
 
 #define MAX_DIGEST_SIZE	64
 #define TASK_COMM_LEN 16
 
 enum errors { UNKNOWN_DIGEST_ERR, CALC_DIGEST_ERR, MMAP_WRITERS_ERR,
-	      MPROTECT_ERR, LAST__ERR };
+	      MPROTECT_ERR, WRITE_MMAPPED_EXEC, LAST__ERR };
 
 struct log_entry {
 	enum errors error;