firmware: replace call to fw_read_file_contents() with kernel version
authorMimi Zohar <zohar@linux.vnet.ibm.com>
Thu, 19 Nov 2015 17:39:22 +0000 (12:39 -0500)
committerMimi Zohar <zohar@linux.vnet.ibm.com>
Sun, 21 Feb 2016 14:03:44 +0000 (09:03 -0500)
Replace the fw_read_file_contents with kernel_file_read_from_path().

Although none of the upstreamed LSMs define a kernel_fw_from_file hook,
IMA is called by the security function to prevent unsigned firmware from
being loaded and to measure/appraise signed firmware, based on policy.

Instead of reading the firmware twice, once for measuring/appraising the
firmware and again for reading the firmware contents into memory, the
kernel_post_read_file() security hook calculates the file hash based on
the in memory file buffer.  The firmware is read once.

This patch removes the LSM kernel_fw_from_file() hook and security call.

Changelog v4+:
- revert dropped buf->size assignment - reported by Sergey Senozhatsky
v3:
- remove kernel_fw_from_file hook
- use kernel_file_read_from_path() - requested by Luis
v2:
- reordered and squashed firmware patches
- fix MAX firmware size (Kees Cook)

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
drivers/base/firmware_class.c
include/linux/fs.h
include/linux/ima.h
include/linux/lsm_hooks.h
include/linux/security.h
security/integrity/ima/ima_main.c
security/security.c

index c743a2f18c337545f68e28e4b5752b3a8b389c2c..a414008ea64c73280278201c6bd7bff016b16c60 100644 (file)
@@ -23,6 +23,7 @@
 #include <linux/sched.h>
 #include <linux/file.h>
 #include <linux/list.h>
+#include <linux/fs.h>
 #include <linux/async.h>
 #include <linux/pm.h>
 #include <linux/suspend.h>
@@ -291,37 +292,6 @@ 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 = 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 void fw_finish_direct_load(struct device *device,
                                  struct firmware_buf *buf)
 {
@@ -334,6 +304,7 @@ static void fw_finish_direct_load(struct device *device,
 static int fw_get_filesystem_firmware(struct device *device,
                                       struct firmware_buf *buf)
 {
+       loff_t size;
        int i, len;
        int rc = -ENOENT;
        char *path;
@@ -343,8 +314,6 @@ static int fw_get_filesystem_firmware(struct device *device,
                return -ENOMEM;
 
        for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
-               struct file *file;
-
                /* skip the unset customized path */
                if (!fw_path[i][0])
                        continue;
@@ -356,18 +325,16 @@ static int fw_get_filesystem_firmware(struct device *device,
                        break;
                }
 
-               file = filp_open(path, O_RDONLY, 0);
-               if (IS_ERR(file))
-                       continue;
-               rc = fw_read_file_contents(file, buf);
-               fput(file);
+               buf->size = 0;
+               rc = kernel_read_file_from_path(path, &buf->data, &size,
+                                               INT_MAX, READING_FIRMWARE);
                if (rc) {
                        dev_warn(device, "loading %s failed with error %d\n",
                                 path, rc);
                        continue;
                }
-               dev_dbg(device, "direct-loading %s\n",
-                       buf->fw_id);
+               dev_dbg(device, "direct-loading %s\n", buf->fw_id);
+               buf->size = size;
                fw_finish_direct_load(device, buf);
                break;
        }
@@ -689,8 +656,9 @@ 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,
+                                               READING_FIRMWARE);
 
                        /*
                         * Same logic as fw_load_abort, only the DONE bit
index 00fa5c45fd63ef9a4bf81d3dca200dc09d2b6680..c8bc4d8c843ff47dc489230eb2e3c1620cb17c47 100644 (file)
@@ -2577,6 +2577,7 @@ static inline void i_readcount_inc(struct inode *inode)
 extern int do_pipe_flags(int *, int);
 
 enum kernel_read_file_id {
+       READING_FIRMWARE = 1,
        READING_MAX_ID
 };
 
index d29a6a23fc19d47e886c8ea1d906cf8b801a8bf3..7aea4863c2442daac70141d2313024e7c0136739 100644 (file)
@@ -19,7 +19,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_post_read_file(struct file *file, void *buf, loff_t size,
                              enum kernel_read_file_id id);
 
@@ -49,11 +48,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_post_read_file(struct file *file, void *buf, loff_t size,
                                     enum kernel_read_file_id id)
 {
index 2337f33913c16f21ff514c93b15864276afed7c5..7d04a1220223424e4c9744d313c08c6b3af1487e 100644 (file)
  *     @inode points to the inode to use as a reference.
  *     The current task must be the one that nominated @inode.
  *     Return 0 if successful.
- * @kernel_fw_from_file:
- *     Load firmware from userspace (not called for built-in firmware).
- *     @file contains the file structure pointing to the file containing
- *     the firmware to load. This argument will be NULL if the firmware
- *     was loaded via the uevent-triggered blob-based interface exposed
- *     by CONFIG_FW_LOADER_USER_HELPER.
- *     @buf pointer to buffer containing firmware contents.
- *     @size length of the firmware contents.
- *     Return 0 if permission is granted.
  * @kernel_module_request:
  *     Ability to trigger the kernel to automatically upcall to userspace for
  *     userspace to load a kernel module with the given name.
@@ -1462,7 +1453,6 @@ union security_list_options {
        void (*cred_transfer)(struct cred *new, const struct cred *old);
        int (*kernel_act_as)(struct cred *new, u32 secid);
        int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
-       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,
@@ -1725,7 +1715,6 @@ struct security_hook_heads {
        struct list_head cred_transfer;
        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;
index d920718dc845292d1a5f973d849fe03449436fbf..cee1349e1155da03158d1e67e5541c391287c7da 100644 (file)
@@ -300,7 +300,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,
@@ -854,12 +853,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;
index 7577653541584a4f04ecb4a64aa5975fd8776919..e9651be17b727b7a160a9e88b23555e4cf83162f 100644 (file)
@@ -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 process_measurement(file, NULL, 0, MAY_EXEC, FIRMWARE_CHECK, 0);
-}
-
 /**
  * ima_post_read_file - in memory collect/appraise/audit measurement
  * @file: pointer to the file to be measured/appraised/audit
@@ -366,12 +355,22 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 {
        enum ima_hooks func = FILE_CHECK;
 
+       if (!file && read_id == READING_FIRMWARE) {
+               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;
                return 0;
        }
 
+       if (read_id == READING_FIRMWARE)
+               func = FIRMWARE_CHECK;
+
        return process_measurement(file, buf, size, MAY_READ, func, 0);
 }
 
index ef4c65a9fd17ac2a6873ae6e4ae7c56adcbeb671..cd85be61c416426bcc5cd5a31b3b5af86872cef4 100644 (file)
@@ -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);
@@ -1703,8 +1692,6 @@ struct security_hook_heads security_hook_heads = {
                LIST_HEAD_INIT(security_hook_heads.kernel_act_as),
        .kernel_create_files_as =
                LIST_HEAD_INIT(security_hook_heads.kernel_create_files_as),
-       .kernel_fw_from_file =
-               LIST_HEAD_INIT(security_hook_heads.kernel_fw_from_file),
        .kernel_module_request =
                LIST_HEAD_INIT(security_hook_heads.kernel_module_request),
        .kernel_module_from_file =