From 16e173d5fc69318ac6414faf191363f10c8da35d Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Wed, 23 Feb 2022 10:45:15 +0100 Subject: [PATCH] Deny further open for writing to files mmapped for execution Commit 3d01ead65846 ("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//fd/ 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 --- ebpf/Makefile.am | 4 +-- ebpf/diglim_kern.c | 58 +++++++++++++++++++++++++++++++------------ ebpf/diglim_user.c | 1 + include/common_kern.h | 2 ++ include/log.h | 3 ++- 5 files changed, 49 insertions(+), 19 deletions(-) diff --git a/ebpf/Makefile.am b/ebpf/Makefile.am index d3cc579..88f2e3b 100644 --- a/ebpf/Makefile.am +++ b/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 diff --git a/ebpf/diglim_kern.c b/ebpf/diglim_kern.c index 9696a4f..2753068 100644 --- a/ebpf/diglim_kern.c +++ b/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; } diff --git a/ebpf/diglim_user.c b/ebpf/diglim_user.c index 8bf57a0..804689a 100644 --- a/ebpf/diglim_user.c +++ b/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) diff --git a/include/common_kern.h b/include/common_kern.h index 1ae0ea7..a86f8ec 100644 --- a/include/common_kern.h +++ b/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 diff --git a/include/log.h b/include/log.h index 0136681..599e23c 100644 --- a/include/log.h +++ b/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;