diff --git a/fs/exec.c b/fs/exec.c index 9f4ba8201b264959f913ac24e8abdda166621536..e328d435be6485caa048a15e8a20576e599b0ca4 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -132,7 +132,7 @@ SYSCALL_DEFINE1(uselib, const char __user *, library) struct filename *tmp = getname(library); int error = PTR_ERR(tmp); static const struct open_flags uselib_flags = { - .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC, + .open_flag = O_LARGEFILE | O_RDONLY, .acc_mode = MAY_READ | MAY_EXEC, .intent = LOOKUP_OPEN, .lookup_flags = LOOKUP_FOLLOW, @@ -913,7 +913,7 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) .lookup_flags = LOOKUP_FOLLOW, }; - if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0) + if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH | AT_CHECK)) != 0) return ERR_PTR(-EINVAL); if (flags & AT_SYMLINK_NOFOLLOW) open_exec_flags.lookup_flags &= ~LOOKUP_FOLLOW; @@ -1496,12 +1496,24 @@ static void free_bprm(struct linux_binprm *bprm) kfree(bprm); } -static struct linux_binprm *alloc_bprm(int fd, struct filename *filename) +static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int flags) { - struct linux_binprm *bprm = kzalloc(sizeof(*bprm), GFP_KERNEL); + struct linux_binprm *bprm; + struct file *file; int retval = -ENOMEM; - if (!bprm) - goto out; + + file = do_open_execat(fd, filename, flags); + if (IS_ERR(file)) + return ERR_CAST(file); + + bprm = kzalloc(sizeof(*bprm), GFP_KERNEL); + if (!bprm) { + allow_write_access(file); + fput(file); + return ERR_PTR(-ENOMEM); + } + + bprm->file = file; if (fd == AT_FDCWD || filename->name[0] == '/') { bprm->filename = filename->name; @@ -1514,18 +1526,42 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename) if (!bprm->fdpath) goto out_free; + /* + * Record that a name derived from an O_CLOEXEC fd will be + * inaccessible after exec. This allows the code in exec to + * choose to fail when the executable is not mmaped into the + * interpreter and an open file descriptor is not passed to + * the interpreter. This makes for a better user experience + * than having the interpreter start and then immediately fail + * when it finds the executable is inaccessible. + */ + if (get_close_on_exec(fd)) + bprm->interp_flags |= BINPRM_FLAGS_PATH_INACCESSIBLE; + bprm->filename = bprm->fdpath; } bprm->interp = bprm->filename; + /* + * At this point, security_file_open() has already been called (with + * __FMODE_EXEC) and access control checks for AT_CHECK will stop just + * after the security_bprm_creds_for_exec() call in bprm_execve(). + * Indeed, the kernel should not try to parse the content of the file + * with exec_binprm() nor change the calling thread, which means that + * the following security functions will be not called: + * - security_bprm_check() + * - security_bprm_creds_from_file() + * - security_bprm_committing_creds() + * - security_bprm_committed_creds() + */ + bprm->is_check = !!(flags & AT_CHECK); + retval = bprm_mm_init(bprm); - if (retval) - goto out_free; - return bprm; + if (!retval) + return bprm; out_free: free_bprm(bprm); -out: return ERR_PTR(retval); } @@ -1793,10 +1829,8 @@ static int exec_binprm(struct linux_binprm *bprm) /* * sys_execve() executes a new program. */ -static int bprm_execve(struct linux_binprm *bprm, - int fd, struct filename *filename, int flags) +static int bprm_execve(struct linux_binprm *bprm) { - struct file *file; int retval; /* @@ -1811,29 +1845,11 @@ static int bprm_execve(struct linux_binprm *bprm, check_unsafe_exec(bprm); current->in_execve = 1; - file = do_open_execat(fd, filename, flags); - retval = PTR_ERR(file); - if (IS_ERR(file)) - goto out_unmark; - sched_exec(); - bprm->file = file; - /* - * Record that a name derived from an O_CLOEXEC fd will be - * inaccessible after exec. This allows the code in exec to - * choose to fail when the executable is not mmaped into the - * interpreter and an open file descriptor is not passed to - * the interpreter. This makes for a better user experience - * than having the interpreter start and then immediately fail - * when it finds the executable is inaccessible. - */ - if (bprm->fdpath && get_close_on_exec(fd)) - bprm->interp_flags |= BINPRM_FLAGS_PATH_INACCESSIBLE; - /* Set the unchanging part of bprm->cred */ retval = security_bprm_creds_for_exec(bprm); - if (retval) + if (retval || bprm->is_check) goto out; retval = exec_binprm(bprm); @@ -1859,7 +1875,6 @@ static int bprm_execve(struct linux_binprm *bprm, if (bprm->point_of_no_return && !fatal_signal_pending(current)) force_sigsegv(SIGSEGV); -out_unmark: current->fs->in_exec = 0; current->in_execve = 0; @@ -1893,7 +1908,7 @@ static int do_execveat_common(int fd, struct filename *filename, * further execve() calls fail. */ current->flags &= ~PF_NPROC_EXCEEDED; - bprm = alloc_bprm(fd, filename); + bprm = alloc_bprm(fd, filename, flags); if (IS_ERR(bprm)) { retval = PTR_ERR(bprm); goto out_ret; @@ -1942,7 +1957,7 @@ static int do_execveat_common(int fd, struct filename *filename, bprm->argc = 1; } - retval = bprm_execve(bprm, fd, filename, flags); + retval = bprm_execve(bprm); out_free: free_bprm(bprm); @@ -1963,7 +1978,7 @@ int kernel_execve(const char *kernel_filename, if (IS_ERR(filename)) return PTR_ERR(filename); - bprm = alloc_bprm(fd, filename); + bprm = alloc_bprm(fd, filename, 0); if (IS_ERR(bprm)) { retval = PTR_ERR(bprm); goto out_ret; @@ -1998,7 +2013,7 @@ int kernel_execve(const char *kernel_filename, if (retval < 0) goto out_free; - retval = bprm_execve(bprm, fd, filename, 0); + retval = bprm_execve(bprm); out_free: free_bprm(bprm); out_ret: diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 1716a07a9809b5e7e0b863d6e850ac6886899125..fff47e750def0ac5766130589058afef9cbb518b 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -42,6 +42,11 @@ struct linux_binprm { * original userspace. */ point_of_no_return:1; + /* + * Set by user space to check executability according to the + * caller's environment. + */ + KABI_FILL_HOLE(unsigned int is_check:1) #ifdef __alpha__ unsigned int taso:1; #endif diff --git a/include/linux/ima.h b/include/linux/ima.h index 99bb44ca711606f6103e5a03ab05281beabe699c..2fef0f3a9c6210038b24ddfdd032fe3b44206996 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -15,6 +15,7 @@ struct linux_binprm; #ifdef CONFIG_IMA extern int ima_bprm_check(struct linux_binprm *bprm); +extern int ima_bprm_creds_for_exec(struct linux_binprm *bprm); extern int ima_file_check(struct file *file, int mask); extern void ima_post_create_tmpfile(struct inode *inode); extern void ima_file_free(struct file *file); @@ -57,6 +58,11 @@ static inline int ima_bprm_check(struct linux_binprm *bprm) return 0; } +static inline int ima_bprm_creds_for_exec(struct linux_binprm *bprm) +{ + return 0; +} + static inline int ima_file_check(struct file *file, int mask) { return 0; diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h index 2f86b2ad6d7e9d6bd7478c369dc301899a37a791..4a4d5a176d5f66fd9655eec4be765eddbd90ef7f 100644 --- a/include/uapi/linux/fcntl.h +++ b/include/uapi/linux/fcntl.h @@ -111,4 +111,35 @@ #define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */ +/* + * AT_CHECK only performs a check on a regular file and returns 0 if execution + * of this file would be allowed, ignoring the file format and then the related + * interpreter dependencies (e.g. ELF libraries, script's shebang). + * + * Programs should always perform this check to apply kernel-level checks + * against files that are not directly executed by the kernel but passed to a + * user space interpreter instead. All files that contain executable code, + * from the point of view of the interpreter, should be checked. However the + * result of this check should only be enforced according to + * SECBIT_EXEC_RESTRICT_FILE or SECBIT_EXEC_DENY_INTERACTIVE. See securebits.h + * documentation and the samples/check-exec/inc.c example. + * + * The main purpose of this flag is to improve the security and consistency of + * an execution environment to ensure that direct file execution (e.g. + * `./script.sh`) and indirect file execution (e.g. `sh script.sh`) lead to the + * same result. For instance, this can be used to check if a file is + * trustworthy according to the caller's environment. + * + * In a secure environment, libraries and any executable dependencies should + * also be checked. For instance, dynamic linking should make sure that all + * libraries are allowed for execution to avoid trivial bypass (e.g. using + * LD_PRELOAD). For such secure execution environment to make sense, only + * trusted code should be executable, which also requires integrity guarantees. + * + * To avoid race conditions leading to time-of-check to time-of-use issues, + * AT_CHECK should be used with AT_EMPTY_PATH to check against a file + * descriptor instead of a path. + */ +#define AT_CHECK 0x10000 + #endif /* _UAPI_LINUX_FCNTL_H */ diff --git a/kernel/audit.h b/kernel/audit.h index 1918019e6aaf7b8c54a248fcc593d22e3ac772c8..59133104ee19089aec3d8edae2c55477cb9e5146 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -187,6 +187,7 @@ struct audit_context { } mmap; struct { int argc; + KABI_FILL_HOLE(bool is_check) } execve; struct { char *name; diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 57b982b44732e48e67a12004e9ef29663c01aca4..f1da45d8afcb5e8f2c0bdebac2af94868a7d7515 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -2403,6 +2403,7 @@ void __audit_bprm(struct linux_binprm *bprm) context->type = AUDIT_EXECVE; context->execve.argc = bprm->argc; + context->execve.is_check = bprm->is_check; } diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 37aa1650c74eb250ede179893eccaf6145ef713d..2a43033f71eba859ec2bb9a52fbb403aad89efcf 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -412,8 +412,10 @@ static int apparmor_file_open(struct file *file) * Cache permissions granted by the previous exec check, with * implicit read and executable mmap which are required to * actually execute the image. + * + * Illogically, FMODE_EXEC is in f_flags, not f_mode. */ - if (current->in_execve) { + if (file->f_flags & __FMODE_EXEC) { fctx->allow = MAY_EXEC | MAY_READ | AA_EXEC_MMAP; return 0; } diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index c93626aa3eb5641ce8303feed2ad271c6394f211..ac5f82b2dd5894688958c6522ca9a725e8e9ac3d 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -597,6 +597,17 @@ int ima_bprm_check(struct linux_binprm *bprm) MAY_EXEC, CREDS_CHECK); } +/** + * ima_bprm_creds_for_exec - ima support exec check. + */ +int ima_bprm_creds_for_exec(struct linux_binprm *bprm) +{ + if (!bprm->is_check) + return 0; + + return ima_bprm_check(bprm); +} + /** * ima_path_check - based on policy, collect/store measurement. * @file: pointer to the file to be measured diff --git a/security/security.c b/security/security.c index 7a01268824667368752ca0a54bd8e229ed307125..a67aa63c8c8b425ca56f6f37b0530b960e6cd902 100644 --- a/security/security.c +++ b/security/security.c @@ -850,9 +850,21 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) return __vm_enough_memory(mm, pages, cap_sys_admin); } +/* + * If execveat(2) is called with the AT_CHECK flag, bprm->is_check is set. The + * result must be the same as without this flag even if the execution will + * never really happen and @bprm will always be dropped. + * + * This hook must not change current->cred, only @bprm->cred. + */ int security_bprm_creds_for_exec(struct linux_binprm *bprm) { - return call_int_hook(bprm_creds_for_exec, 0, bprm); + int ret; + + ret = call_int_hook(bprm_creds_for_exec, 0, bprm); + if (ret) + return ret; + return ima_bprm_creds_for_exec(bprm); } int security_bprm_creds_from_file(struct linux_binprm *bprm, struct file *file) @@ -1643,6 +1655,11 @@ int security_file_receive(struct file *file) return call_int_hook(file_receive, 0, file); } +/* + * We can check if a file is opened for execution (e.g. execve(2) call), either + * directly or indirectly (e.g. ELF's ld.so) by checking file->f_flags & + * __FMODE_EXEC . + */ int security_file_open(struct file *file) { int ret; diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c index a8dc3ae938f9c222212e9540a9b9862ec767007b..90fd5300d2f5fedd04ebf96ba21a9e3321014b2f 100644 --- a/security/tomoyo/tomoyo.c +++ b/security/tomoyo/tomoyo.c @@ -308,7 +308,8 @@ static int tomoyo_file_fcntl(struct file *file, unsigned int cmd, static int tomoyo_file_open(struct file *f) { /* Don't check read permission here if called from execve(). */ - if (current->in_execve) + /* Illogically, FMODE_EXEC is in f_flags, not f_mode. */ + if (f->f_flags & __FMODE_EXEC) return 0; return tomoyo_check_open_permission(tomoyo_domain(), &f->f_path, f->f_flags);