Commit e40ba6d5 authored by Mimi Zohar's avatar Mimi Zohar
Browse files

firmware: replace call to fw_read_file_contents() with kernel version 



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: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
Acked-by: default avatarKees Cook <keescook@chromium.org>
Acked-by: default avatarLuis R. Rodriguez <mcgrof@kernel.org>
parent 09596b94

drivers/base/firmware_class.c

+10 −42
Original line number Diff line number Diff line
@@ -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

include/linux/fs.h

+1 −0
Original line number Diff line number Diff line
@@ -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

};

include/linux/ima.h

+0 −6
Original line number Diff line number Diff line
@@ -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)

{

include/linux/lsm_hooks.h

+0 −11
Original line number Diff line number Diff line
@@ -541,15 +541,6 @@ 
 *	@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;

include/linux/security.h

+0 −7
Original line number Diff line number Diff line
@@ -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;

CRA Git | Maintained and supported by SUSTech CRA and CCSE