This patch replaces the module copy_module_from_fd() call with the VFS
common kernel_read_file_from_fd() function. Instead of reading the
kernel module twice, once for measuring/appraising and then loading
the kernel module, the file is read once.
This patch defines a new security hook named security_kernel_read_file(),
which is called before reading the file. For now, call the module
security hook from security_kernel_read_file until the LSMs have been
converted to use the kernel_read_file hook.
This patch retains the kernel_module_from_file hook, but removes the
security_kernel_module_from_file() function.
Signed-off-by: Mimi Zohar <zohar at linux.vnet.ibm.com>
---
fs/exec.c | 4 +++
include/linux/ima.h | 1 +
include/linux/lsm_hooks.h | 8 +++++
include/linux/security.h | 3 +-
kernel/module.c | 67 ++++-------------------------------
security/integrity/ima/ima.h | 1 -
security/integrity/ima/ima_appraise.c | 7 ----
security/integrity/ima/ima_main.c | 5 ++-
security/integrity/ima/ima_policy.c | 16 ++++-----
security/integrity/integrity.h | 12 +++----
security/security.c | 12 +++++--
11 files changed, 47 insertions(+), 89 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index a5ae51e..3524e5f 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -842,6 +842,10 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
if (!S_ISREG(file_inode(file)->i_mode))
return -EINVAL;
+ ret = security_kernel_read_file(file, policy_id);
+ if (ret)
+ return ret;
+
i_size = i_size_read(file_inode(file));
if (max_size > 0 && i_size > max_size)
return -EFBIG;
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0a7f039..eec5e2b 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -17,6 +17,7 @@ enum ima_policy_id {
KEXEC_CHECK = 1,
INITRAMFS_CHECK,
FIRMWARE_CHECK,
+ MODULE_CHECK,
IMA_MAX_READ_CHECK
};
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 4e6e2af..9915310 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -561,6 +561,12 @@
* the kernel module to load. If the module is being loaded from a blob,
* this argument will be NULL.
* Return 0 if permission is granted.
+ * @kernel_read_file:
+ * Read a file specified by userspace.
+ * @file contains the file structure pointing to the file being read
+ * by the kernel.
+ * @policy_id contains IMA policy identifier.
+ * Return 0 if permission is granted.
* @kernel_post_read_file:
* Read a file specified by userspace.
* @file contains the file structure pointing to the file being read
@@ -1465,6 +1471,7 @@ union security_list_options {
int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
int (*kernel_module_request)(char *kmod_name);
int (*kernel_module_from_file)(struct file *file);
+ int (*kernel_read_file)(struct file *file, int policy_id);
int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
int policy_id);
int (*task_fix_setuid)(struct cred *new, const struct cred *old,
@@ -1726,6 +1733,7 @@ struct security_hook_heads {
struct list_head kernel_act_as;
struct list_head kernel_create_files_as;
struct list_head kernel_fw_from_file;
+ struct list_head kernel_read_file;
struct list_head kernel_post_read_file;
struct list_head kernel_module_request;
struct list_head kernel_module_from_file;
diff --git a/include/linux/security.h b/include/linux/security.h
index 51f3bc6..6d005b3 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -301,6 +301,7 @@ int security_kernel_act_as(struct cred *new, u32 secid);
int security_kernel_create_files_as(struct cred *new, struct inode *inode);
int security_kernel_module_request(char *kmod_name);
int security_kernel_module_from_file(struct file *file);
+int security_kernel_read_file(struct file *file, int policy_id);
int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
int policy_id);
int security_task_fix_setuid(struct cred *new, const struct cred *old,
@@ -857,7 +858,7 @@ static inline int security_kernel_module_request(char *kmod_name)
return 0;
}
-static inline int security_kernel_module_from_file(struct file *file)
+static inline int security_kernel_read_file(struct file *file, int policy_id)
{
return 0;
}
diff --git a/kernel/module.c b/kernel/module.c
index 8f051a1..7398d12 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2665,7 +2665,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
if (info->len < sizeof(*(info->hdr)))
return -ENOEXEC;
- err = security_kernel_module_from_file(NULL);
+ err = security_kernel_read_file(NULL, MODULE_CHECK);
if (err)
return err;
@@ -2683,63 +2683,6 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
return 0;
}
-/* Sets info->hdr and info->len. */
-static int copy_module_from_fd(int fd, struct load_info *info)
-{
- struct fd f = fdget(fd);
- int err;
- struct kstat stat;
- loff_t pos;
- ssize_t bytes = 0;
-
- if (!f.file)
- return -ENOEXEC;
-
- err = security_kernel_module_from_file(f.file);
- if (err)
- goto out;
-
- err = vfs_getattr(&f.file->f_path, &stat);
- if (err)
- goto out;
-
- if (stat.size > INT_MAX) {
- err = -EFBIG;
- goto out;
- }
-
- /* Don't hand 0 to vmalloc, it whines. */
- if (stat.size == 0) {
- err = -EINVAL;
- goto out;
- }
-
- info->hdr = vmalloc(stat.size);
- if (!info->hdr) {
- err = -ENOMEM;
- goto out;
- }
-
- pos = 0;
- while (pos < stat.size) {
- bytes = kernel_read(f.file, pos, (char *)(info->hdr) + pos,
- stat.size - pos);
- if (bytes < 0) {
- vfree(info->hdr);
- err = bytes;
- goto out;
- }
- if (bytes == 0)
- break;
- pos += bytes;
- }
- info->len = pos;
-
-out:
- fdput(f);
- return err;
-}
-
static void free_copy(struct load_info *info)
{
vfree(info->hdr);
@@ -3602,8 +3545,10 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
{
- int err;
struct load_info info = { };
+ loff_t size;
+ void *hdr;
+ int err;
err = may_init_module();
if (err)
@@ -3615,9 +3560,11 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
|MODULE_INIT_IGNORE_VERMAGIC))
return -EINVAL;
- err = copy_module_from_fd(fd, &info);
+ err = kernel_read_file_from_fd(fd, &hdr, &size, INT_MAX, MODULE_CHECK);
if (err)
return err;
+ info.hdr = hdr;
+ info.len = size;
return load_module(&info, uargs, flags);
}
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 520c7b4..fc31ba2 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -164,7 +164,6 @@ enum ima_hooks {
FILE_CHECK = IMA_MAX_READ_CHECK,
MMAP_CHECK,
BPRM_CHECK,
- MODULE_CHECK,
POST_SETATTR
};
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 57b1ad1..6b3e30a 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -74,8 +74,6 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
return iint->ima_mmap_status;
case BPRM_CHECK:
return iint->ima_bprm_status;
- case MODULE_CHECK:
- return iint->ima_module_status;
case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
return iint->ima_read_status;
case FILE_CHECK:
@@ -94,8 +92,6 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
case BPRM_CHECK:
iint->ima_bprm_status = status;
break;
- case MODULE_CHECK:
- iint->ima_module_status = status;
break;
case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
iint->ima_read_status = status;
@@ -116,9 +112,6 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func)
case BPRM_CHECK:
iint->flags |= (IMA_BPRM_APPRAISED | IMA_APPRAISED);
break;
- case MODULE_CHECK:
- iint->flags |= (IMA_MODULE_APPRAISED | IMA_APPRAISED);
- break;
case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
break;
case FILE_CHECK:
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 1251882..107e6a7 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -334,7 +334,7 @@ int ima_module_check(struct file *file)
#endif
return 0; /* We rely on module signature checking */
}
- return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0);
+ return 0;
}
/**
@@ -357,6 +357,9 @@ int ima_hash_and_process_file(struct file *file, void *buf, loff_t size,
return 0;
}
+ if (!file && policy_id == MODULE_CHECK) /* MODULE_SIG_FORCE enabled */
+ return 0;
+
if (!file || !buf || size == 0) { /* should never happen */
if (ima_appraise & IMA_APPRAISE_ENFORCE)
return -EACCES;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index dbd7aa1..dbfd26b 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -101,7 +101,7 @@ static struct ima_rule_entry original_measurement_rules[] = {
.flags = IMA_FUNC | IMA_MASK},
{.action = MEASURE, .hooks.func = FILE_CHECK, .mask = MAY_READ,
.uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_MASK | IMA_UID},
- {.action = MEASURE, .hooks.func = MODULE_CHECK, .flags = IMA_FUNC},
+ {.action = MEASURE, .hooks.policy_id = MODULE_CHECK, .flags = IMA_FUNC},
{.action = MEASURE, .hooks.policy_id = FIRMWARE_CHECK,
.flags = IMA_FUNC},
};
@@ -304,8 +304,6 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
return IMA_MMAP_APPRAISE;
case BPRM_CHECK:
return IMA_BPRM_APPRAISE;
- case MODULE_CHECK:
- return IMA_MODULE_APPRAISE;
case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
return IMA_READ_APPRAISE;
case FILE_CHECK:
@@ -607,8 +605,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
/* PATH_CHECK is for backwards compat */
else if (strcmp(args[0].from, "PATH_CHECK") == 0)
entry->hooks.func = FILE_CHECK;
- else if (strcmp(args[0].from, "MODULE_CHECK") == 0)
- entry->hooks.func = MODULE_CHECK;
else if ((strcmp(args[0].from, "FILE_MMAP") == 0)
|| (strcmp(args[0].from, "MMAP_CHECK") == 0))
entry->hooks.func = MMAP_CHECK;
@@ -620,6 +616,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->hooks.policy_id = INITRAMFS_CHECK;
else if (strcmp(args[0].from, "FIRMWARE_CHECK") == 0)
entry->hooks.policy_id = FIRMWARE_CHECK;
+ else if (strcmp(args[0].from, "MODULE_CHECK") == 0)
+ entry->hooks.policy_id = MODULE_CHECK;
else
result = -EINVAL;
if (!result)
@@ -774,7 +772,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
}
if (!result && (entry->action == UNKNOWN))
result = -EINVAL;
- else if (entry->hooks.func == MODULE_CHECK)
+ else if (entry->hooks.policy_id == MODULE_CHECK)
temp_ima_appraise |= IMA_APPRAISE_MODULES;
else if (entry->hooks.policy_id == FIRMWARE_CHECK)
temp_ima_appraise |= IMA_APPRAISE_FIRMWARE;
@@ -946,9 +944,6 @@ int ima_policy_show(struct seq_file *m, void *v)
case BPRM_CHECK:
seq_printf(m, pt(Opt_func), ft(func_bprm));
break;
- case MODULE_CHECK:
- seq_printf(m, pt(Opt_func), ft(func_module));
- break;
case POST_SETATTR:
seq_printf(m, pt(Opt_func), ft(func_post));
break;
@@ -963,6 +958,9 @@ int ima_policy_show(struct seq_file *m, void *v)
case FIRMWARE_CHECK:
seq_printf(m, pt(Opt_func), ft(func_firmware));
break;
+ case MODULE_CHECK:
+ seq_printf(m, pt(Opt_func), ft(func_module));
+ break;
default:
snprintf(tbuf, sizeof(tbuf), "%d",
entry->hooks.func);
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 75334cd..97fb5c2 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -45,16 +45,12 @@
#define IMA_MMAP_APPRAISED 0x00000800
#define IMA_BPRM_APPRAISE 0x00001000
#define IMA_BPRM_APPRAISED 0x00002000
-#define IMA_MODULE_APPRAISE 0x00004000
-#define IMA_MODULE_APPRAISED 0x00008000
-#define IMA_READ_APPRAISE 0x00010000
-#define IMA_READ_APPRAISED 0x00020000
+#define IMA_READ_APPRAISE 0x00004000
+#define IMA_READ_APPRAISED 0x00008000
#define IMA_APPRAISE_SUBMASK (IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
- IMA_BPRM_APPRAISE | IMA_MODULE_APPRAISE | \
- IMA_READ_APPRAISE)
+ IMA_BPRM_APPRAISE | IMA_READ_APPRAISE)
#define IMA_APPRAISED_SUBMASK (IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
- IMA_BPRM_APPRAISED | IMA_MODULE_APPRAISED | \
- IMA_READ_APPRAISED)
+ IMA_BPRM_APPRAISED | IMA_READ_APPRAISED)
enum evm_ima_xattr_type {
IMA_XATTR_DIGEST = 0x01,
diff --git a/security/security.c b/security/security.c
index a391ce4..fa8a9e8 100644
--- a/security/security.c
+++ b/security/security.c
@@ -889,11 +889,17 @@ int security_kernel_module_request(char *kmod_name)
return call_int_hook(kernel_module_request, 0, kmod_name);
}
-int security_kernel_module_from_file(struct file *file)
+int security_kernel_read_file(struct file *file, int policy_id)
{
int ret;
- ret = call_int_hook(kernel_module_from_file, 0, file);
+ switch (policy_id) {
+ case MODULE_CHECK:
+ ret = call_int_hook(kernel_module_from_file, 0, file);
+ break;
+ default:
+ ret = call_int_hook(kernel_read_file, 0, file, policy_id);
+ }
if (ret)
return ret;
return ima_module_check(file);
@@ -1707,6 +1713,8 @@ struct security_hook_heads security_hook_heads = {
LIST_HEAD_INIT(security_hook_heads.kernel_module_request),
.kernel_module_from_file =
LIST_HEAD_INIT(security_hook_heads.kernel_module_from_file),
+ .kernel_read_file =
+ LIST_HEAD_INIT(security_hook_heads.kernel_read_file),
.kernel_post_read_file =
LIST_HEAD_INIT(security_hook_heads.kernel_post_read_file),
.task_fix_setuid =
--
2.1.0
