* [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1)
@ 2016-01-08 19:21 Mimi Zohar
2016-01-08 19:22 ` [RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel Mimi Zohar
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Mimi Zohar @ 2016-01-08 19:21 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, Luis R. Rodriguez, kexec, linux-modules, fsdevel,
David Howells, David Woodhouse, Kees Cook, Dmitry Torokhov
For a while it was looked down upon to directly read files from Linux.
These days there exists a few mechanisms in the kernel that do just this
though to load a file into a local buffer. There are minor but important
checks differences on each, we should take all the best practices from
each of them, generalize them and make all places in the kernel that
read a file use it.[1]
One difference is the method for opening the file. In some cases we
have a file, while in other cases we have a pathname or a file descriptor.
Another difference is the security hook calls, or lack of them. In
some versions there is a post file read hook, while in others there
is a pre file read hook.
This patch set is the first attempt at resolving these differences. It
does not attempt to merge the different methods of opening a file, but
defines a single common kernel file read function with two wrappers.
Although this patch set defines two new security hooks for pre and post
file read, it does not attempt to merge the existing security hooks.
That is left as future work.
These patches are based on top of the "ima: measuring/appraising files
read by the kernel". The latest version of these patches can be found
in the next-kernel-read branch of:
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
Mimi Zohar (5):
vfs: define a generic function to read a file from the kernel
firmware: replace call to fw_read_file_contents() with kernel version
kexec: replace call to copy_file_from_fd() with kernel version
ima: replace call to integrity_read_file() with kernel version
module: replace copy_module_from_fd with kernel version
drivers/base/firmware_class.c | 51 +++++--------------
fs/exec.c | 96 +++++++++++++++++++++++++++++++++++
include/linux/fs.h | 3 ++
include/linux/ima.h | 7 +--
include/linux/lsm_hooks.h | 19 +++++++
include/linux/security.h | 14 +++--
kernel/kexec_file.c | 76 +++------------------------
kernel/module.c | 67 +++---------------------
security/integrity/ima/ima.h | 1 -
security/integrity/ima/ima_appraise.c | 7 ---
security/integrity/ima/ima_fs.c | 15 +++---
security/integrity/ima/ima_main.c | 21 ++++----
security/integrity/ima/ima_policy.c | 16 +++---
security/integrity/integrity.h | 12 ++---
security/security.c | 46 ++++++++++++-----
15 files changed, 217 insertions(+), 234 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel
2016-01-08 19:21 [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1) Mimi Zohar
@ 2016-01-08 19:22 ` Mimi Zohar
2016-01-08 20:24 ` Kees Cook
2016-01-08 19:22 ` [RFC PATCH 2/5] firmware: replace call to fw_read_file_contents() with kernel version Mimi Zohar
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Mimi Zohar @ 2016-01-08 19:22 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, Luis R. Rodriguez, kexec, linux-modules, fsdevel,
David Howells, David Woodhouse, Kees Cook, Dmitry Torokhov
In order to measure and appraise files being read by the kernel,
new module and kexec syscalls were defined which include a file
descriptor. Other places in the kernel (eg. firmware, IMA,
sound) also read files.
This patch introduces a common function for reading files from
the kernel with the corresponding security post-read hook and
function.
Changelog:
- Add missing <linux/vmalloc.h>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
fs/exec.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fs.h | 1 +
include/linux/lsm_hooks.h | 11 ++++++++++
include/linux/security.h | 9 ++++++++
security/security.c | 16 ++++++++++++++
5 files changed, 93 insertions(+)
diff --git a/fs/exec.c b/fs/exec.c
index b06623a..3c48a19 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -56,6 +56,7 @@
#include <linux/pipe_fs_i.h>
#include <linux/oom.h>
#include <linux/compat.h>
+#include <linux/vmalloc.h>
#include <asm/uaccess.h>
#include <asm/mmu_context.h>
@@ -831,6 +832,61 @@ int kernel_read(struct file *file, loff_t offset,
EXPORT_SYMBOL(kernel_read);
+int kernel_read_file(struct file *file, void **buf, loff_t *size,
+ loff_t max_size, int policy_id)
+{
+ loff_t i_size, pos;
+ ssize_t bytes = 0;
+ int ret;
+
+ if (!S_ISREG(file_inode(file)->i_mode))
+ return -EINVAL;
+
+ i_size = i_size_read(file_inode(file));
+ if (max_size > 0 && i_size > max_size)
+ return -EFBIG;
+ if (i_size == 0)
+ return -EINVAL;
+
+ *buf = vmalloc(i_size);
+ if (!*buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ pos = 0;
+ while (pos < i_size) {
+ bytes = kernel_read(file, pos, (char *)(*buf) + pos,
+ i_size - pos);
+ if (bytes < 0) {
+ ret = bytes;
+ goto out_free;
+ }
+
+ if (bytes == 0)
+ break;
+ pos += bytes;
+ }
+
+ if (pos != i_size) {
+ ret = -EBADF; /* firmware uses -EIO */
+ goto out_free;
+ }
+
+ ret = security_kernel_post_read_file(file, *buf, i_size, policy_id);
+ if (!ret)
+ *size = pos;
+
+out_free:
+ if (ret < 0) {
+ vfree(*buf);
+ *buf = NULL;
+ }
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file);
+
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3aa5142..9b1468c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2527,6 +2527,7 @@ static inline void i_readcount_inc(struct inode *inode)
extern int do_pipe_flags(int *, int);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
+extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
extern struct file * open_exec(const char *);
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 71969de..10baa8f 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -561,6 +561,14 @@
* 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_post_read_file:
+ * Read a file specified by userspace.
+ * @file contains the file structure pointing to the file being read
+ * by the kernel.
+ * @buf pointer to buffer containing the file contents.
+ * @size length of the file contents.
+ * @policy_id contains the calling function identifier.
+ * Return 0 if permission is granted.
* @task_fix_setuid:
* Update the module's state after setting one or more of the user
* identity attributes of the current process. The @flags parameter
@@ -1457,6 +1465,8 @@ 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_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,
int flags);
int (*task_setpgid)(struct task_struct *p, pid_t pgid);
@@ -1716,6 +1726,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_post_read_file;
struct list_head kernel_module_request;
struct list_head kernel_module_from_file;
struct list_head task_fix_setuid;
diff --git a/include/linux/security.h b/include/linux/security.h
index 4824a4c..44d8832 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -301,6 +301,8 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode);
int security_kernel_fw_from_file(struct file *file, char *buf, size_t size);
int security_kernel_module_request(char *kmod_name);
int security_kernel_module_from_file(struct file *file);
+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,
int flags);
int security_task_setpgid(struct task_struct *p, pid_t pgid);
@@ -866,6 +868,13 @@ static inline int security_kernel_module_from_file(struct file *file)
return 0;
}
+static inline int security_kernel_post_read_file(struct file *file,
+ char *buf, loff_t size,
+ int policy_id)
+{
+ return 0;
+}
+
static inline int security_task_fix_setuid(struct cred *new,
const struct cred *old,
int flags)
diff --git a/security/security.c b/security/security.c
index e8ffd92..e979c2d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -910,6 +910,20 @@ int security_kernel_module_from_file(struct file *file)
return ima_module_check(file);
}
+int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
+ int policy_id)
+{
+ int ret;
+
+ ret = call_int_hook(kernel_post_read_file, 0, file, buf, size,
+ policy_id);
+ if (ret)
+ return ret;
+
+ return ima_hash_and_process_file(file, buf, size, policy_id);
+}
+EXPORT_SYMBOL_GPL(security_kernel_post_read_file);
+
int security_task_fix_setuid(struct cred *new, const struct cred *old,
int flags)
{
@@ -1697,6 +1711,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_post_read_file =
+ LIST_HEAD_INIT(security_hook_heads.kernel_post_read_file),
.task_fix_setuid =
LIST_HEAD_INIT(security_hook_heads.task_fix_setuid),
.task_setpgid = LIST_HEAD_INIT(security_hook_heads.task_setpgid),
--
2.1.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFC PATCH 2/5] firmware: replace call to fw_read_file_contents() with kernel version
2016-01-08 19:21 [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1) Mimi Zohar
2016-01-08 19:22 ` [RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel Mimi Zohar
@ 2016-01-08 19:22 ` Mimi Zohar
2016-01-08 20:26 ` Kees Cook
2016-01-08 19:22 ` [RFC PATCH 3/5] kexec: replace call to copy_file_from_fd() " Mimi Zohar
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Mimi Zohar @ 2016-01-08 19:22 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, Luis R. Rodriguez, kexec, linux-modules, fsdevel,
David Howells, David Woodhouse, Kees Cook, Dmitry Torokhov
Replace fw_read_file_contents() for reading a file with the common VFS
kernel_read_file() function. Call the existing firmware security hook
from security_kernel_post_read_file() until the LSMs have been converted.
This patch retains the kernel_fw_from_file() hook, but removes the
security_kernel_fw_from_file() function.
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
drivers/base/firmware_class.c | 51 +++++++++------------------------------
include/linux/ima.h | 6 -----
include/linux/security.h | 8 +-----
security/integrity/ima/ima_main.c | 18 ++++++--------
security/security.c | 24 ++++++++----------
5 files changed, 30 insertions(+), 77 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 3ca96a6..4e4e860 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -292,44 +292,10 @@ static const char * const fw_path[] = {
module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path");
-static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
-{
- int size;
- char *buf;
- int rc;
-
- if (!S_ISREG(file_inode(file)->i_mode))
- return -EINVAL;
- size = i_size_read(file_inode(file));
- if (size <= 0)
- return -EINVAL;
- buf = vmalloc(size);
- if (!buf)
- return -ENOMEM;
- rc = kernel_read(file, 0, buf, size);
- if (rc != size) {
- if (rc > 0)
- rc = -EIO;
- goto fail;
- }
- rc = ima_hash_and_process_file(file, buf, size, FIRMWARE_CHECK);
- if (rc)
- goto fail;
-
- rc = security_kernel_fw_from_file(file, buf, size);
- if (rc)
- goto fail;
- fw_buf->data = buf;
- fw_buf->size = size;
- return 0;
-fail:
- vfree(buf);
- return rc;
-}
-
static int fw_get_filesystem_firmware(struct device *device,
struct firmware_buf *buf)
{
+ loff_t size;
int i, len;
int rc = -ENOENT;
char *path;
@@ -355,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device *device,
file = filp_open(path, O_RDONLY, 0);
if (IS_ERR(file))
continue;
- rc = fw_read_file_contents(file, buf);
+
+ buf->size = 0;
+ rc = kernel_read_file(file, &buf->data, &size, UINT_MAX,
+ FIRMWARE_CHECK);
fput(file);
if (rc)
dev_warn(device, "firmware, attempted to load %s, but failed with error %d\n",
path, rc);
- else
+ else {
+ buf->size = (size_t) size;
break;
+ }
}
__putname(path);
@@ -690,8 +661,10 @@ static ssize_t firmware_loading_store(struct device *dev,
dev_err(dev, "%s: map pages failed\n",
__func__);
else
- rc = security_kernel_fw_from_file(NULL,
- fw_buf->data, fw_buf->size);
+ rc = security_kernel_post_read_file(NULL,
+ fw_buf->data,
+ fw_buf->size,
+ FIRMWARE_CHECK);
/*
* Same logic as fw_load_abort, only the DONE bit
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 3790af0..7cad2e7 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -27,7 +27,6 @@ extern int ima_file_check(struct file *file, int mask, int opened);
extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long prot);
extern int ima_module_check(struct file *file);
-extern int ima_fw_from_file(struct file *file, char *buf, size_t size);
extern int ima_hash_and_process_file(struct file *file,
void *buf, loff_t size,
enum ima_policy_id policy_id);
@@ -58,11 +57,6 @@ static inline int ima_module_check(struct file *file)
return 0;
}
-static inline int ima_fw_from_file(struct file *file, char *buf, size_t size)
-{
- return 0;
-}
-
static inline int ima_hash_and_process_file(struct file *file,
void *buf, loff_t size,
enum ima_policy_id policy_id)
diff --git a/include/linux/security.h b/include/linux/security.h
index 44d8832..51f3bc6 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -28,6 +28,7 @@
#include <linux/err.h>
#include <linux/string.h>
#include <linux/mm.h>
+#include <linux/ima.h>
struct linux_binprm;
struct cred;
@@ -298,7 +299,6 @@ int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
void security_transfer_creds(struct cred *new, const struct cred *old);
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_fw_from_file(struct file *file, char *buf, size_t size);
int security_kernel_module_request(char *kmod_name);
int security_kernel_module_from_file(struct file *file);
int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
@@ -852,12 +852,6 @@ static inline int security_kernel_create_files_as(struct cred *cred,
return 0;
}
-static inline int security_kernel_fw_from_file(struct file *file,
- char *buf, size_t size)
-{
- return 0;
-}
-
static inline int security_kernel_module_request(char *kmod_name)
{
return 0;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index bfe1cc3..62d609d 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -337,17 +337,6 @@ int ima_module_check(struct file *file)
return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0);
}
-int ima_fw_from_file(struct file *file, char *buf, size_t size)
-{
- if (!file) {
- if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
- (ima_appraise & IMA_APPRAISE_ENFORCE))
- return -EACCES; /* INTEGRITY_UNKNOWN */
- return 0;
- }
- return 0;
-}
-
/**
* ima_hash_and_process_file - in memory collect/appraise/audit measurement
* @file: pointer to the file to be measured/appraised/audit
@@ -362,6 +351,13 @@ int ima_fw_from_file(struct file *file, char *buf, size_t size)
int ima_hash_and_process_file(struct file *file, void *buf, loff_t size,
enum ima_policy_id policy_id)
{
+ if (!file && policy_id == FIRMWARE_CHECK) {
+ if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
+ (ima_appraise & IMA_APPRAISE_ENFORCE))
+ return -EACCES; /* INTEGRITY_UNKNOWN */
+ return 0;
+ }
+
if (!file || !buf || size == 0) { /* should never happen */
if (ima_appraise & IMA_APPRAISE_ENFORCE)
return -EACCES;
diff --git a/security/security.c b/security/security.c
index e979c2d..a391ce4 100644
--- a/security/security.c
+++ b/security/security.c
@@ -884,17 +884,6 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode)
return call_int_hook(kernel_create_files_as, 0, new, inode);
}
-int security_kernel_fw_from_file(struct file *file, char *buf, size_t size)
-{
- int ret;
-
- ret = call_int_hook(kernel_fw_from_file, 0, file, buf, size);
- if (ret)
- return ret;
- return ima_fw_from_file(file, buf, size);
-}
-EXPORT_SYMBOL_GPL(security_kernel_fw_from_file);
-
int security_kernel_module_request(char *kmod_name)
{
return call_int_hook(kernel_module_request, 0, kmod_name);
@@ -913,10 +902,17 @@ int security_kernel_module_from_file(struct file *file)
int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
int policy_id)
{
- int ret;
+ int ret = 0;
- ret = call_int_hook(kernel_post_read_file, 0, file, buf, size,
- policy_id);
+ switch (policy_id) {
+ case FIRMWARE_CHECK:
+ ret = call_int_hook(kernel_fw_from_file, 0, file, buf, size);
+ break;
+ default:
+ ret = call_int_hook(kernel_post_read_file, 0, file, buf, size,
+ policy_id);
+ break;
+ }
if (ret)
return ret;
--
2.1.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFC PATCH 3/5] kexec: replace call to copy_file_from_fd() with kernel version
2016-01-08 19:21 [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1) Mimi Zohar
2016-01-08 19:22 ` [RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel Mimi Zohar
2016-01-08 19:22 ` [RFC PATCH 2/5] firmware: replace call to fw_read_file_contents() with kernel version Mimi Zohar
@ 2016-01-08 19:22 ` Mimi Zohar
2016-01-08 19:22 ` [RFC PATCH 4/5] ima: replace call to integrity_read_file() " Mimi Zohar
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Mimi Zohar @ 2016-01-08 19:22 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, Luis R. Rodriguez, kexec, linux-modules, fsdevel,
David Howells, David Woodhouse, Kees Cook, Dmitry Torokhov
This patch defines kernel_read_file_from_fd(), a wrapper for the VFS
common kernel_read_file(), and replaces the kexec copy_file_from_fd()
calls with the kernel_read_file_from_fd() wrapper.
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
fs/exec.c | 15 +++++++++++
include/linux/fs.h | 1 +
kernel/kexec_file.c | 76 +++++------------------------------------------------
3 files changed, 23 insertions(+), 69 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 3c48a19..4ad2fca 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -887,6 +887,21 @@ out:
}
EXPORT_SYMBOL_GPL(kernel_read_file);
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+ int policy_id)
+{
+ struct fd f = fdget(fd);
+ int ret = -ENOEXEC;
+
+ if (!f.file)
+ goto out;
+
+ ret = kernel_read_file(f.file, buf, size, max_size, policy_id);
+out:
+ fdput(f);
+ return ret;
+}
+
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9b1468c..9642623 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2528,6 +2528,7 @@ extern int do_pipe_flags(int *, int);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
+extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t, int);
extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
extern struct file * open_exec(const char *);
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 81d20e8..f7c3ce4 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -34,69 +34,6 @@ size_t __weak kexec_purgatory_size = 0;
static int kexec_calculate_store_digests(struct kimage *image);
-static int copy_file_from_fd(int fd, void **buf, unsigned long *buf_len,
- enum ima_policy_id policy_id)
-{
- struct fd f = fdget(fd);
- int ret;
- struct kstat stat;
- loff_t pos;
- ssize_t bytes = 0;
-
- if (!f.file)
- return -EBADF;
-
- ret = vfs_getattr(&f.file->f_path, &stat);
- if (ret)
- goto out;
-
- if (stat.size > INT_MAX) {
- ret = -EFBIG;
- goto out;
- }
-
- /* Don't hand 0 to vmalloc, it whines. */
- if (stat.size == 0) {
- ret = -EINVAL;
- goto out;
- }
-
- *buf = vmalloc(stat.size);
- if (!*buf) {
- ret = -ENOMEM;
- goto out;
- }
-
- pos = 0;
- while (pos < stat.size) {
- bytes = kernel_read(f.file, pos, (char *)(*buf) + pos,
- stat.size - pos);
- if (bytes < 0) {
- ret = bytes;
- goto out_free;
- }
-
- if (bytes == 0)
- break;
- pos += bytes;
- }
-
- if (pos != stat.size)
- ret = -EBADF;
-
- ret = ima_hash_and_process_file(f.file, *buf, stat.size, policy_id);
- if (!ret)
- *buf_len = pos;
-out_free:
- if (ret < 0) {
- vfree(*buf);
- *buf = NULL;
- }
-out:
- fdput(f);
- return ret;
-}
-
/* Architectures can provide this probe function */
int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
unsigned long buf_len)
@@ -185,16 +122,17 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
{
int ret = 0;
void *ldata;
+ loff_t size;
- ret = copy_file_from_fd(kernel_fd, &image->kernel_buf,
- &image->kernel_buf_len, KEXEC_CHECK);
+ ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
+ &size, INT_MAX, KEXEC_CHECK);
if (ret)
return ret;
+ image->kernel_buf_len = size;
/* Call arch image probe handlers */
ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
image->kernel_buf_len);
-
if (ret)
goto out;
@@ -209,11 +147,11 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
#endif
/* It is possible that there no initramfs is being loaded */
if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
- ret = copy_file_from_fd(initrd_fd, &image->initrd_buf,
- &image->initrd_buf_len,
- INITRAMFS_CHECK);
+ ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
+ &size, INT_MAX, INITRAMFS_CHECK);
if (ret)
goto out;
+ image->initrd_buf_len = size;
}
if (cmdline_len) {
--
2.1.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFC PATCH 4/5] ima: replace call to integrity_read_file() with kernel version
2016-01-08 19:21 [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1) Mimi Zohar
` (2 preceding siblings ...)
2016-01-08 19:22 ` [RFC PATCH 3/5] kexec: replace call to copy_file_from_fd() " Mimi Zohar
@ 2016-01-08 19:22 ` Mimi Zohar
2016-01-08 19:22 ` [RFC PATCH 5/5] module: replace copy_module_from_fd " Mimi Zohar
2016-01-08 19:32 ` [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1) Mimi Zohar
5 siblings, 0 replies; 11+ messages in thread
From: Mimi Zohar @ 2016-01-08 19:22 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, Luis R. Rodriguez, kexec, linux-modules, fsdevel,
David Howells, David Woodhouse, Kees Cook, Dmitry Torokhov
This patch defines kernel_read_file_from_path(), a wrapper for the VFS
common kernel_read_file(), and replaces the integrity_read_file() with
a call to the kernel_read_file_from_path() wrapper.
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
fs/exec.c | 21 +++++++++++++++++++++
include/linux/fs.h | 1 +
security/integrity/ima/ima_fs.c | 15 +++++++++------
3 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 4ad2fca..f79c845 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -902,6 +902,27 @@ out:
return ret;
}
+int kernel_read_file_from_path(char *path, void **buf, loff_t *size,
+ loff_t max_size, int policy_id)
+{
+ struct file *file;
+ int ret;
+
+ if (!path || !*path)
+ return -EINVAL;
+
+ file = filp_open(path, O_RDONLY, 0);
+ if (IS_ERR(file)) {
+ ret = PTR_ERR(file);
+ pr_err("Unable to open file: %s (%d)", path, ret);
+ return ret;
+ }
+
+ ret = kernel_read_file(file, buf, size, max_size, policy_id);
+ fput(file);
+ return ret;
+}
+
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9642623..79b1172 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2529,6 +2529,7 @@ extern int do_pipe_flags(int *, int);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t, int);
+extern int kernel_read_file_from_path(char *, void **, loff_t *, loff_t, int);
extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
extern struct file * open_exec(const char *);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 685fdca..80bc30b 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -22,6 +22,7 @@
#include <linux/rculist.h>
#include <linux/rcupdate.h>
#include <linux/parser.h>
+#include <linux/vmalloc.h>
#include "ima.h"
@@ -260,20 +261,22 @@ static const struct file_operations ima_ascii_measurements_ops = {
static ssize_t ima_read_policy(char *path)
{
- char *data, *datap;
- int rc, size, pathlen = strlen(path);
+ void *data;
+ char *datap;
+ loff_t size;
+ int rc, pathlen = strlen(path);
+
char *p;
/* remove \n */
datap = path;
strsep(&datap, "\n");
- rc = integrity_read_file(path, &data, POLICY_CHECK);
+ rc = kernel_read_file_from_path(path, &data, &size, 0, POLICY_CHECK);
if (rc < 0)
return rc;
- size = rc;
- datap = data;
+ datap = (char *)data;
while (size > 0 && (p = strsep(&datap, "\n"))) {
pr_debug("rule: %s\n", p);
rc = ima_parse_add_rule(p);
@@ -282,7 +285,7 @@ static ssize_t ima_read_policy(char *path)
size -= rc;
}
- kfree(data);
+ vfree(data);
if (rc < 0)
return rc;
else if (size)
--
2.1.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFC PATCH 5/5] module: replace copy_module_from_fd with kernel version
2016-01-08 19:21 [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1) Mimi Zohar
` (3 preceding siblings ...)
2016-01-08 19:22 ` [RFC PATCH 4/5] ima: replace call to integrity_read_file() " Mimi Zohar
@ 2016-01-08 19:22 ` Mimi Zohar
2016-01-08 19:32 ` [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1) Mimi Zohar
5 siblings, 0 replies; 11+ messages in thread
From: Mimi Zohar @ 2016-01-08 19:22 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, Luis R. Rodriguez, kexec, linux-modules, fsdevel,
David Howells, David Woodhouse, Kees Cook, Dmitry Torokhov
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@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 f79c845..f251371 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 7cad2e7..969552b 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -18,6 +18,7 @@ enum ima_policy_id {
INITRAMFS_CHECK,
FIRMWARE_CHECK,
POLICY_CHECK,
+ MODULE_CHECK,
IMA_MAX_READ_CHECK
};
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 10baa8f..206a225 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 the calling function 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 c6ff5c8..e8f111b 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 62d609d..415ee21 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;
}
/**
@@ -358,6 +358,9 @@ int ima_hash_and_process_file(struct file *file, void *buf, loff_t size,
return 0;
}
+ if (!file && policy_id == FIRMWARE_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 dcc0e6b..bf30acf 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},
};
@@ -309,8 +309,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:
@@ -615,8 +613,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;
@@ -630,6 +626,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->hooks.policy_id = FIRMWARE_CHECK;
else if (strcmp(args[0].from, "POLICY_CHECK") == 0)
entry->hooks.policy_id = POLICY_CHECK;
+ else if (strcmp(args[0].from, "MODULE_CHECK") == 0)
+ entry->hooks.policy_id = MODULE_CHECK;
else
result = -EINVAL;
if (!result)
@@ -784,7 +782,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;
@@ -960,9 +958,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;
@@ -980,6 +975,9 @@ int ima_policy_show(struct seq_file *m, void *v)
case POLICY_CHECK:
seq_printf(m, pt(Opt_func), ft(func_policy));
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 9986edd..76fe25d 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -46,16 +46,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1)
2016-01-08 19:21 [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1) Mimi Zohar
` (4 preceding siblings ...)
2016-01-08 19:22 ` [RFC PATCH 5/5] module: replace copy_module_from_fd " Mimi Zohar
@ 2016-01-08 19:32 ` Mimi Zohar
5 siblings, 0 replies; 11+ messages in thread
From: Mimi Zohar @ 2016-01-08 19:32 UTC (permalink / raw)
To: linux-security-module
Cc: Luis R. Rodriguez, kexec, linux-modules, fsdevel, David Howells,
David Woodhouse, Kees Cook, Dmitry Torokhov
On Fri, 2016-01-08 at 14:21 -0500, Mimi Zohar wrote:
> For a while it was looked down upon to directly read files from Linux.
> These days there exists a few mechanisms in the kernel that do just this
> though to load a file into a local buffer. There are minor but important
> checks differences on each, we should take all the best practices from
> each of them, generalize them and make all places in the kernel that
> read a file use it.[1]
>
> One difference is the method for opening the file. In some cases we
> have a file, while in other cases we have a pathname or a file descriptor.
>
> Another difference is the security hook calls, or lack of them. In
> some versions there is a post file read hook, while in others there
> is a pre file read hook.
>
> This patch set is the first attempt at resolving these differences. It
> does not attempt to merge the different methods of opening a file, but
> defines a single common kernel file read function with two wrappers.
> Although this patch set defines two new security hooks for pre and post
> file read, it does not attempt to merge the existing security hooks.
> That is left as future work.
>
> These patches are based on top of the "ima: measuring/appraising files
> read by the kernel". The latest version of these patches can be found
> in the next-kernel-read branch of:
> git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
[1] Taken from Luis Rodriguez's wiki -
http://kernelnewbies.org/KernelProjects/common-kernel-loader
Mimi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel
2016-01-08 19:22 ` [RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel Mimi Zohar
@ 2016-01-08 20:24 ` Kees Cook
2016-01-08 20:29 ` Mimi Zohar
0 siblings, 1 reply; 11+ messages in thread
From: Kees Cook @ 2016-01-08 20:24 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, Luis R. Rodriguez, Kexec Mailing List,
linux-modules, linux-fsdevel@vger.kernel.org, David Howells,
David Woodhouse, Dmitry Torokhov
On Fri, Jan 8, 2016 at 11:22 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> In order to measure and appraise files being read by the kernel,
> new module and kexec syscalls were defined which include a file
> descriptor. Other places in the kernel (eg. firmware, IMA,
> sound) also read files.
>
> This patch introduces a common function for reading files from
> the kernel with the corresponding security post-read hook and
> function.
>
> Changelog:
> - Add missing <linux/vmalloc.h>
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
> fs/exec.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/fs.h | 1 +
> include/linux/lsm_hooks.h | 11 ++++++++++
> include/linux/security.h | 9 ++++++++
> security/security.c | 16 ++++++++++++++
> 5 files changed, 93 insertions(+)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index b06623a..3c48a19 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -56,6 +56,7 @@
> #include <linux/pipe_fs_i.h>
> #include <linux/oom.h>
> #include <linux/compat.h>
> +#include <linux/vmalloc.h>
>
> #include <asm/uaccess.h>
> #include <asm/mmu_context.h>
> @@ -831,6 +832,61 @@ int kernel_read(struct file *file, loff_t offset,
>
> EXPORT_SYMBOL(kernel_read);
>
> +int kernel_read_file(struct file *file, void **buf, loff_t *size,
> + loff_t max_size, int policy_id)
> +{
> + loff_t i_size, pos;
> + ssize_t bytes = 0;
> + int ret;
> +
> + if (!S_ISREG(file_inode(file)->i_mode))
> + return -EINVAL;
> +
> + i_size = i_size_read(file_inode(file));
> + if (max_size > 0 && i_size > max_size)
> + return -EFBIG;
> + if (i_size == 0)
> + return -EINVAL;
> +
> + *buf = vmalloc(i_size);
This could get very large -- what risks do we have to system stability
here? Having userspace able to trigger such a massive allocation could
be a problem. The firmware loader was limited to MAX_INT...
-Kees
> + if (!*buf) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + pos = 0;
> + while (pos < i_size) {
> + bytes = kernel_read(file, pos, (char *)(*buf) + pos,
> + i_size - pos);
> + if (bytes < 0) {
> + ret = bytes;
> + goto out_free;
> + }
> +
> + if (bytes == 0)
> + break;
> + pos += bytes;
> + }
> +
> + if (pos != i_size) {
> + ret = -EBADF; /* firmware uses -EIO */
> + goto out_free;
> + }
> +
> + ret = security_kernel_post_read_file(file, *buf, i_size, policy_id);
> + if (!ret)
> + *size = pos;
> +
> +out_free:
> + if (ret < 0) {
> + vfree(*buf);
> + *buf = NULL;
> + }
> +out:
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(kernel_read_file);
> +
> ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
> {
> ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 3aa5142..9b1468c 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2527,6 +2527,7 @@ static inline void i_readcount_inc(struct inode *inode)
> extern int do_pipe_flags(int *, int);
>
> extern int kernel_read(struct file *, loff_t, char *, unsigned long);
> +extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
> extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
> extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
> extern struct file * open_exec(const char *);
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 71969de..10baa8f 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -561,6 +561,14 @@
> * 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_post_read_file:
> + * Read a file specified by userspace.
> + * @file contains the file structure pointing to the file being read
> + * by the kernel.
> + * @buf pointer to buffer containing the file contents.
> + * @size length of the file contents.
> + * @policy_id contains the calling function identifier.
> + * Return 0 if permission is granted.
> * @task_fix_setuid:
> * Update the module's state after setting one or more of the user
> * identity attributes of the current process. The @flags parameter
> @@ -1457,6 +1465,8 @@ 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_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,
> int flags);
> int (*task_setpgid)(struct task_struct *p, pid_t pgid);
> @@ -1716,6 +1726,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_post_read_file;
> struct list_head kernel_module_request;
> struct list_head kernel_module_from_file;
> struct list_head task_fix_setuid;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 4824a4c..44d8832 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -301,6 +301,8 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode);
> int security_kernel_fw_from_file(struct file *file, char *buf, size_t size);
> int security_kernel_module_request(char *kmod_name);
> int security_kernel_module_from_file(struct file *file);
> +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,
> int flags);
> int security_task_setpgid(struct task_struct *p, pid_t pgid);
> @@ -866,6 +868,13 @@ static inline int security_kernel_module_from_file(struct file *file)
> return 0;
> }
>
> +static inline int security_kernel_post_read_file(struct file *file,
> + char *buf, loff_t size,
> + int policy_id)
> +{
> + return 0;
> +}
> +
> static inline int security_task_fix_setuid(struct cred *new,
> const struct cred *old,
> int flags)
> diff --git a/security/security.c b/security/security.c
> index e8ffd92..e979c2d 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -910,6 +910,20 @@ int security_kernel_module_from_file(struct file *file)
> return ima_module_check(file);
> }
>
> +int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
> + int policy_id)
> +{
> + int ret;
> +
> + ret = call_int_hook(kernel_post_read_file, 0, file, buf, size,
> + policy_id);
> + if (ret)
> + return ret;
> +
> + return ima_hash_and_process_file(file, buf, size, policy_id);
> +}
> +EXPORT_SYMBOL_GPL(security_kernel_post_read_file);
> +
> int security_task_fix_setuid(struct cred *new, const struct cred *old,
> int flags)
> {
> @@ -1697,6 +1711,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_post_read_file =
> + LIST_HEAD_INIT(security_hook_heads.kernel_post_read_file),
> .task_fix_setuid =
> LIST_HEAD_INIT(security_hook_heads.task_fix_setuid),
> .task_setpgid = LIST_HEAD_INIT(security_hook_heads.task_setpgid),
> --
> 2.1.0
>
--
Kees Cook
Chrome OS & Brillo Security
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 2/5] firmware: replace call to fw_read_file_contents() with kernel version
2016-01-08 19:22 ` [RFC PATCH 2/5] firmware: replace call to fw_read_file_contents() with kernel version Mimi Zohar
@ 2016-01-08 20:26 ` Kees Cook
2016-01-08 20:36 ` Mimi Zohar
0 siblings, 1 reply; 11+ messages in thread
From: Kees Cook @ 2016-01-08 20:26 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, Luis R. Rodriguez, Kexec Mailing List,
linux-modules, linux-fsdevel@vger.kernel.org, David Howells,
David Woodhouse, Dmitry Torokhov
On Fri, Jan 8, 2016 at 11:22 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Replace fw_read_file_contents() for reading a file with the common VFS
> kernel_read_file() function. Call the existing firmware security hook
> from security_kernel_post_read_file() until the LSMs have been converted.
>
> This patch retains the kernel_fw_from_file() hook, but removes the
> security_kernel_fw_from_file() function.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
> drivers/base/firmware_class.c | 51 +++++++++------------------------------
> include/linux/ima.h | 6 -----
> include/linux/security.h | 8 +-----
> security/integrity/ima/ima_main.c | 18 ++++++--------
> security/security.c | 24 ++++++++----------
> 5 files changed, 30 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 3ca96a6..4e4e860 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -292,44 +292,10 @@ static const char * const fw_path[] = {
> module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
> MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path");
>
> -static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
> -{
> - int size;
> - char *buf;
> - int rc;
> -
> - if (!S_ISREG(file_inode(file)->i_mode))
> - return -EINVAL;
> - size = i_size_read(file_inode(file));
> - if (size <= 0)
> - return -EINVAL;
> - buf = vmalloc(size);
> - if (!buf)
> - return -ENOMEM;
> - rc = kernel_read(file, 0, buf, size);
> - if (rc != size) {
> - if (rc > 0)
> - rc = -EIO;
> - goto fail;
> - }
> - rc = ima_hash_and_process_file(file, buf, size, FIRMWARE_CHECK);
> - if (rc)
> - goto fail;
> -
> - rc = security_kernel_fw_from_file(file, buf, size);
> - if (rc)
> - goto fail;
> - fw_buf->data = buf;
> - fw_buf->size = size;
> - return 0;
> -fail:
> - vfree(buf);
> - return rc;
> -}
> -
> static int fw_get_filesystem_firmware(struct device *device,
> struct firmware_buf *buf)
> {
> + loff_t size;
> int i, len;
> int rc = -ENOENT;
> char *path;
> @@ -355,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device *device,
> file = filp_open(path, O_RDONLY, 0);
> if (IS_ERR(file))
> continue;
> - rc = fw_read_file_contents(file, buf);
> +
> + buf->size = 0;
> + rc = kernel_read_file(file, &buf->data, &size, UINT_MAX,
Strictly speaking, the originally code would max at INT_MAX, no UINT_MAX.
-Kees
> + FIRMWARE_CHECK);
> fput(file);
> if (rc)
> dev_warn(device, "firmware, attempted to load %s, but failed with error %d\n",
> path, rc);
> - else
> + else {
> + buf->size = (size_t) size;
> break;
> + }
> }
> __putname(path);
>
> @@ -690,8 +661,10 @@ static ssize_t firmware_loading_store(struct device *dev,
> dev_err(dev, "%s: map pages failed\n",
> __func__);
> else
> - rc = security_kernel_fw_from_file(NULL,
> - fw_buf->data, fw_buf->size);
> + rc = security_kernel_post_read_file(NULL,
> + fw_buf->data,
> + fw_buf->size,
> + FIRMWARE_CHECK);
>
> /*
> * Same logic as fw_load_abort, only the DONE bit
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 3790af0..7cad2e7 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -27,7 +27,6 @@ extern int ima_file_check(struct file *file, int mask, int opened);
> extern void ima_file_free(struct file *file);
> extern int ima_file_mmap(struct file *file, unsigned long prot);
> extern int ima_module_check(struct file *file);
> -extern int ima_fw_from_file(struct file *file, char *buf, size_t size);
> extern int ima_hash_and_process_file(struct file *file,
> void *buf, loff_t size,
> enum ima_policy_id policy_id);
> @@ -58,11 +57,6 @@ static inline int ima_module_check(struct file *file)
> return 0;
> }
>
> -static inline int ima_fw_from_file(struct file *file, char *buf, size_t size)
> -{
> - return 0;
> -}
> -
> static inline int ima_hash_and_process_file(struct file *file,
> void *buf, loff_t size,
> enum ima_policy_id policy_id)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 44d8832..51f3bc6 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -28,6 +28,7 @@
> #include <linux/err.h>
> #include <linux/string.h>
> #include <linux/mm.h>
> +#include <linux/ima.h>
>
> struct linux_binprm;
> struct cred;
> @@ -298,7 +299,6 @@ int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
> void security_transfer_creds(struct cred *new, const struct cred *old);
> 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_fw_from_file(struct file *file, char *buf, size_t size);
> int security_kernel_module_request(char *kmod_name);
> int security_kernel_module_from_file(struct file *file);
> int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
> @@ -852,12 +852,6 @@ static inline int security_kernel_create_files_as(struct cred *cred,
> return 0;
> }
>
> -static inline int security_kernel_fw_from_file(struct file *file,
> - char *buf, size_t size)
> -{
> - return 0;
> -}
> -
> static inline int security_kernel_module_request(char *kmod_name)
> {
> return 0;
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index bfe1cc3..62d609d 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -337,17 +337,6 @@ int ima_module_check(struct file *file)
> return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0);
> }
>
> -int ima_fw_from_file(struct file *file, char *buf, size_t size)
> -{
> - if (!file) {
> - if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> - (ima_appraise & IMA_APPRAISE_ENFORCE))
> - return -EACCES; /* INTEGRITY_UNKNOWN */
> - return 0;
> - }
> - return 0;
> -}
> -
> /**
> * ima_hash_and_process_file - in memory collect/appraise/audit measurement
> * @file: pointer to the file to be measured/appraised/audit
> @@ -362,6 +351,13 @@ int ima_fw_from_file(struct file *file, char *buf, size_t size)
> int ima_hash_and_process_file(struct file *file, void *buf, loff_t size,
> enum ima_policy_id policy_id)
> {
> + if (!file && policy_id == FIRMWARE_CHECK) {
> + if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> + (ima_appraise & IMA_APPRAISE_ENFORCE))
> + return -EACCES; /* INTEGRITY_UNKNOWN */
> + return 0;
> + }
> +
> if (!file || !buf || size == 0) { /* should never happen */
> if (ima_appraise & IMA_APPRAISE_ENFORCE)
> return -EACCES;
> diff --git a/security/security.c b/security/security.c
> index e979c2d..a391ce4 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -884,17 +884,6 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode)
> return call_int_hook(kernel_create_files_as, 0, new, inode);
> }
>
> -int security_kernel_fw_from_file(struct file *file, char *buf, size_t size)
> -{
> - int ret;
> -
> - ret = call_int_hook(kernel_fw_from_file, 0, file, buf, size);
> - if (ret)
> - return ret;
> - return ima_fw_from_file(file, buf, size);
> -}
> -EXPORT_SYMBOL_GPL(security_kernel_fw_from_file);
> -
> int security_kernel_module_request(char *kmod_name)
> {
> return call_int_hook(kernel_module_request, 0, kmod_name);
> @@ -913,10 +902,17 @@ int security_kernel_module_from_file(struct file *file)
> int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
> int policy_id)
> {
> - int ret;
> + int ret = 0;
>
> - ret = call_int_hook(kernel_post_read_file, 0, file, buf, size,
> - policy_id);
> + switch (policy_id) {
> + case FIRMWARE_CHECK:
> + ret = call_int_hook(kernel_fw_from_file, 0, file, buf, size);
> + break;
> + default:
> + ret = call_int_hook(kernel_post_read_file, 0, file, buf, size,
> + policy_id);
> + break;
> + }
> if (ret)
> return ret;
>
> --
> 2.1.0
>
--
Kees Cook
Chrome OS & Brillo Security
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel
2016-01-08 20:24 ` Kees Cook
@ 2016-01-08 20:29 ` Mimi Zohar
0 siblings, 0 replies; 11+ messages in thread
From: Mimi Zohar @ 2016-01-08 20:29 UTC (permalink / raw)
To: Kees Cook
Cc: linux-security-module, Luis R. Rodriguez, Kexec Mailing List,
linux-modules, linux-fsdevel@vger.kernel.org, David Howells,
David Woodhouse, Dmitry Torokhov
On Fri, 2016-01-08 at 12:24 -0800, Kees Cook wrote:
> On Fri, Jan 8, 2016 at 11:22 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > In order to measure and appraise files being read by the kernel,
> > new module and kexec syscalls were defined which include a file
> > descriptor. Other places in the kernel (eg. firmware, IMA,
> > sound) also read files.
> >
> > This patch introduces a common function for reading files from
> > the kernel with the corresponding security post-read hook and
> > function.
> >
> > Changelog:
> > - Add missing <linux/vmalloc.h>
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > ---
> > fs/exec.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/fs.h | 1 +
> > include/linux/lsm_hooks.h | 11 ++++++++++
> > include/linux/security.h | 9 ++++++++
> > security/security.c | 16 ++++++++++++++
> > 5 files changed, 93 insertions(+)
> >
> > diff --git a/fs/exec.c b/fs/exec.c
> > index b06623a..3c48a19 100644
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -56,6 +56,7 @@
> > #include <linux/pipe_fs_i.h>
> > #include <linux/oom.h>
> > #include <linux/compat.h>
> > +#include <linux/vmalloc.h>
> >
> > #include <asm/uaccess.h>
> > #include <asm/mmu_context.h>
> > @@ -831,6 +832,61 @@ int kernel_read(struct file *file, loff_t offset,
> >
> > EXPORT_SYMBOL(kernel_read);
> >
> > +int kernel_read_file(struct file *file, void **buf, loff_t *size,
> > + loff_t max_size, int policy_id)
> > +{
> > + loff_t i_size, pos;
> > + ssize_t bytes = 0;
> > + int ret;
> > +
> > + if (!S_ISREG(file_inode(file)->i_mode))
> > + return -EINVAL;
> > +
> > + i_size = i_size_read(file_inode(file));
> > + if (max_size > 0 && i_size > max_size)
> > + return -EFBIG;
> > + if (i_size == 0)
> > + return -EINVAL;
> > +
> > + *buf = vmalloc(i_size);
>
> This could get very large -- what risks do we have to system stability
> here? Having userspace able to trigger such a massive allocation could
> be a problem. The firmware loader was limited to MAX_INT...
The different callers allowed different sizes. Instead of hard coding
the max size for all callers, the third parameter of kernel_file_read is
the caller max_size.
Mimi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 2/5] firmware: replace call to fw_read_file_contents() with kernel version
2016-01-08 20:26 ` Kees Cook
@ 2016-01-08 20:36 ` Mimi Zohar
0 siblings, 0 replies; 11+ messages in thread
From: Mimi Zohar @ 2016-01-08 20:36 UTC (permalink / raw)
To: Kees Cook
Cc: linux-security-module, Luis R. Rodriguez, Kexec Mailing List,
linux-modules, linux-fsdevel@vger.kernel.org, David Howells,
David Woodhouse, Dmitry Torokhov
On Fri, 2016-01-08 at 12:26 -0800, Kees Cook wrote:
> On Fri, Jan 8, 2016 at 11:22 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Replace fw_read_file_contents() for reading a file with the common VFS
> > kernel_read_file() function. Call the existing firmware security hook
> > from security_kernel_post_read_file() until the LSMs have been converted.
> >
> > This patch retains the kernel_fw_from_file() hook, but removes the
> > security_kernel_fw_from_file() function.
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > ---
> > drivers/base/firmware_class.c | 51 +++++++++------------------------------
> > include/linux/ima.h | 6 -----
> > include/linux/security.h | 8 +-----
> > security/integrity/ima/ima_main.c | 18 ++++++--------
> > security/security.c | 24 ++++++++----------
> > 5 files changed, 30 insertions(+), 77 deletions(-)
> >
> > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> > index 3ca96a6..4e4e860 100644
> > --- a/drivers/base/firmware_class.c
> > +++ b/drivers/base/firmware_class.c
> > @@ -292,44 +292,10 @@ static const char * const fw_path[] = {
> > module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
> > MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path");
> >
> > -static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
> > -{
> > - int size;
> > - char *buf;
> > - int rc;
> > -
> > - if (!S_ISREG(file_inode(file)->i_mode))
> > - return -EINVAL;
> > - size = i_size_read(file_inode(file));
> > - if (size <= 0)
> > - return -EINVAL;
> > - buf = vmalloc(size);
> > - if (!buf)
> > - return -ENOMEM;
> > - rc = kernel_read(file, 0, buf, size);
> > - if (rc != size) {
> > - if (rc > 0)
> > - rc = -EIO;
> > - goto fail;
> > - }
> > - rc = ima_hash_and_process_file(file, buf, size, FIRMWARE_CHECK);
> > - if (rc)
> > - goto fail;
> > -
> > - rc = security_kernel_fw_from_file(file, buf, size);
> > - if (rc)
> > - goto fail;
> > - fw_buf->data = buf;
> > - fw_buf->size = size;
> > - return 0;
> > -fail:
> > - vfree(buf);
> > - return rc;
> > -}
> > -
> > static int fw_get_filesystem_firmware(struct device *device,
> > struct firmware_buf *buf)
> > {
> > + loff_t size;
> > int i, len;
> > int rc = -ENOENT;
> > char *path;
> > @@ -355,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device *device,
> > file = filp_open(path, O_RDONLY, 0);
> > if (IS_ERR(file))
> > continue;
> > - rc = fw_read_file_contents(file, buf);
> > +
> > + buf->size = 0;
> > + rc = kernel_read_file(file, &buf->data, &size, UINT_MAX,
>
> Strictly speaking, the originally code would max at INT_MAX, no UINT_MAX.
hm, I must have taken it from firmware_buf->size, which is defined as
size_t (unsigned). Thanks for the correction.
Mimi
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-01-08 20:37 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-08 19:21 [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1) Mimi Zohar
2016-01-08 19:22 ` [RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel Mimi Zohar
2016-01-08 20:24 ` Kees Cook
2016-01-08 20:29 ` Mimi Zohar
2016-01-08 19:22 ` [RFC PATCH 2/5] firmware: replace call to fw_read_file_contents() with kernel version Mimi Zohar
2016-01-08 20:26 ` Kees Cook
2016-01-08 20:36 ` Mimi Zohar
2016-01-08 19:22 ` [RFC PATCH 3/5] kexec: replace call to copy_file_from_fd() " Mimi Zohar
2016-01-08 19:22 ` [RFC PATCH 4/5] ima: replace call to integrity_read_file() " Mimi Zohar
2016-01-08 19:22 ` [RFC PATCH 5/5] module: replace copy_module_from_fd " Mimi Zohar
2016-01-08 19:32 ` [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1) Mimi Zohar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).