IMA has support for validating files during execution using the
bprm_check functionality. However, this is called before new credentials
have been applied to the task. The previous patch in this series added
an additional IMA check in the bprm_committed_creds hook - however, if a
file is handled by binfmt_misc or binfmt_script (or, in an extremely
unlikely corner case, binfmt_em86), only the interpreter will be
appraised since bprm_committed_creds is never called for the initial
executable.
This patch adds an additional bprm_interpreted hook and calls it from
the end of the relevant binfmt implementations. It is effectively
identical to the bprm_committed_creds hook, except that bprm->file
points to the initial file rather than to the interpreter.
Signed-off-by: Matthew Garrett <mjg59@xxxxxxxxxx>
Cc: James Morris <james.l.morris@xxxxxxxxxx>
Cc: Kees Cook <keescook@xxxxxxxxxxxx>
Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
---
fs/binfmt_em86.c | 31 ++++++++++++++++++++++---------
fs/binfmt_misc.c | 11 +++++++----
fs/binfmt_script.c | 45 +++++++++++++++++++++++++++++++--------------
include/linux/lsm_hooks.h | 7 +++++++
include/linux/security.h | 1 +
security/security.c | 11 +++++++++++
6 files changed, 79 insertions(+), 27 deletions(-)
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index dd2d3f0cd55d..e954ec123d27 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -26,7 +26,7 @@ static int load_em86(struct linux_binprm *bprm)
{
const char *i_name, *i_arg;
char *interp;
- struct file * file;
+ struct file *file, *old;
int retval;
struct elfhdr elf_ex;
@@ -47,8 +47,7 @@ static int load_em86(struct linux_binprm *bprm)
if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE)
return -ENOENT;
- allow_write_access(bprm->file);
- fput(bprm->file);
+ old = bprm->file;
bprm->file = NULL;
/* Unlike in the script case, we don't have to do any hairy
@@ -72,11 +71,13 @@ static int load_em86(struct linux_binprm *bprm)
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ goto ret;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ goto ret;
bprm->argc++;
/*
@@ -85,16 +86,28 @@ static int load_em86(struct linux_binprm *bprm)
* space, and we don't need to copy it.
*/
file = open_exec(interp);
- if (IS_ERR(file))
- return PTR_ERR(file);
+ if (IS_ERR(file)) {
+ retval = PTR_ERR(file);
+ goto ret;
+ }
bprm->file = file;
retval = prepare_binprm(bprm);
if (retval < 0)
- return retval;
+ goto ret;
- return search_binary_handler(bprm);
+ retval = search_binary_handler(bprm);
+ if (retval < 0)
+ goto ret;
+
+ bprm->file = old;
+ retval = security_bprm_interpreted(bprm);
+ bprm->file = file;
+ret:
+ allow_write_access(old);
+ fput(old);
+ return retval;
}
static struct linux_binfmt em86_format = {
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 2a46762def31..81e6e02348f9 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -130,7 +130,7 @@ static Node *check_file(struct linux_binprm *bprm)
static int load_misc_binary(struct linux_binprm *bprm)
{
Node *fmt;
- struct file *interp_file = NULL;
+ struct file *interp_file = NULL, *old = bprm->file;
int retval;
int fd_binary = -1;
@@ -175,7 +175,6 @@ static int load_misc_binary(struct linux_binprm *bprm)
regardless of the interpreter's permissions */
would_dump(bprm, bprm->file);
- allow_write_access(bprm->file);
bprm->file = NULL;
/* mark the bprm that fd should be passed to interp */
@@ -183,8 +182,6 @@ static int load_misc_binary(struct linux_binprm *bprm)
bprm->interp_data = fd_binary;
} else {
- allow_write_access(bprm->file);
- fput(bprm->file);
bprm->file = NULL;
}
/* make argv[1] be the path to the binary */
@@ -236,7 +233,13 @@ static int load_misc_binary(struct linux_binprm *bprm)
if (retval < 0)
goto error;
+ bprm->file = old;
+ retval = security_bprm_interpreted(bprm);
+ bprm->file = interp_file;
ret:
+ allow_write_access(old);
+ if (fd_binary < 0)
+ fput(old);
dput(fmt->dentry);
return retval;
error:
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 7cde3f46ad26..f2bd2aa15702 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -13,12 +13,13 @@
#include <linux/file.h>
#include <linux/err.h>
#include <linux/fs.h>
+#include <linux/security.h>
static int load_script(struct linux_binprm *bprm)
{
const char *i_arg, *i_name;
char *cp;
- struct file *file;
+ struct file *file, *old;
int retval;
if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!'))
@@ -38,8 +39,7 @@ static int load_script(struct linux_binprm *bprm)
* Sorta complicated, but hopefully it will work. -TYT
*/
- allow_write_access(bprm->file);
- fput(bprm->file);
+ old = bprm->file;
bprm->file = NULL;
bprm->buf[BINPRM_BUF_SIZE - 1] = '\0';
@@ -54,8 +54,11 @@ static int load_script(struct linux_binprm *bprm)
break;
}
for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
- if (*cp == '\0')
- return -ENOEXEC; /* No interpreter name found */
+ if (*cp == '\0') {
+ retval = -ENOEXEC; /* No interpreter name found */
+ goto ret;
+ }
+
i_name = cp;
i_arg = NULL;
for ( ; *cp && (*cp != ' ') && (*cp != '\t'); cp++)
@@ -76,37 +79,51 @@ static int load_script(struct linux_binprm *bprm)
*/
retval = remove_arg_zero(bprm);
if (retval)
- return retval;
+ goto ret;
retval = copy_strings_kernel(1, &bprm->interp, bprm);
if (retval < 0)
- return retval;
+ goto ret;
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
if (retval < 0)
- return retval;
+ goto ret;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
if (retval)
- return retval;
+ goto ret;
bprm->argc++;
retval = bprm_change_interp(i_name, bprm);
if (retval < 0)
- return retval;
+ goto ret;
/*
* OK, now restart the process with the interpreter's dentry.
*/
file = open_exec(i_name);
- if (IS_ERR(file))
- return PTR_ERR(file);
+ if (IS_ERR(file)) {
+ retval = PTR_ERR(file);
+ goto ret;
+ }
bprm->file = file;
retval = prepare_binprm(bprm);
if (retval < 0)
- return retval;
- return search_binary_handler(bprm);
+ goto ret;
+ retval = search_binary_handler(bprm);
+ if (retval)
+ goto ret;
+ /*
+ * Allow for validation of the script as well as the interpreter
+ */
+ bprm->file = old;
+ retval = security_bprm_interpreted(bprm);
+ bprm->file = file;
+ret:
+ allow_write_access(old);
+ fput(old);
+ return retval;
}
static struct linux_binfmt script_format = {
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index c9258124e417..fd23b4098098 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -75,6 +75,11 @@
* linux_binprm structure. This hook is a good place to perform state
* changes on the process such as clearing out non-inheritable signal
* state. This is called immediately after commit_creds().
+ * @bprm_interpreted:
+ * Validate the credentials of an executable that was interpreted via
+ * binfmt_misc or binfmt_script. This occurs after the new credentials
+ * have been committed to @current->cred, but @bprm->file points to the
+ * original file rather than the interpreter that was used to execute it.
*
* Security hooks for filesystem operations.
*
@@ -1383,6 +1388,7 @@ union security_list_options {
int (*bprm_check_security)(struct linux_binprm *bprm);
void (*bprm_committing_creds)(struct linux_binprm *bprm);
void (*bprm_committed_creds)(struct linux_binprm *bprm);
+ int (*bprm_interpreted)(struct linux_binprm *bprm);
int (*sb_alloc_security)(struct super_block *sb);
void (*sb_free_security)(struct super_block *sb);
@@ -1703,6 +1709,7 @@ struct security_hook_heads {
struct list_head bprm_check_security;
struct list_head bprm_committing_creds;
struct list_head bprm_committed_creds;
+ struct list_head bprm_interpreted;
struct list_head sb_alloc_security;
struct list_head sb_free_security;
struct list_head sb_copy_data;
diff --git a/include/linux/security.h b/include/linux/security.h
index ad970a4f19f6..e48c38379c64 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -233,6 +233,7 @@ int security_bprm_set_creds(struct linux_binprm *bprm);
int security_bprm_check(struct linux_binprm *bprm);
void security_bprm_committing_creds(struct linux_binprm *bprm);
int security_bprm_committed_creds(struct linux_binprm *bprm);
+int security_bprm_interpreted(struct linux_binprm *bprm);
int security_sb_alloc(struct super_block *sb);
void security_sb_free(struct super_block *sb);
int security_sb_copy_data(char *orig, char *copy);
diff --git a/security/security.c b/security/security.c
index c2c5017e3477..6b9cb108ff61 100644
--- a/security/security.c
+++ b/security/security.c
@@ -352,6 +352,17 @@ int security_bprm_committed_creds(struct linux_binprm *bprm)
return ima_creds_check(bprm);
}
+int security_bprm_interpreted(struct linux_binprm *bprm)
+{
+ int ret;
+
+ ret = call_int_hook(bprm_interpreted, 0, bprm);
+ if (ret)
+ return ret;
+
+ return ima_creds_check(bprm);
+}
+
int security_sb_alloc(struct super_block *sb)
{
return call_int_hook(sb_alloc_security, 0, sb);
--
2.15.0.rc0.271.g36b669edcc-goog
