Skip to content

Commit e1043d8

Browse files
keesgregkh
authored andcommitted
fs/kernel_read_file: Remove FIRMWARE_PREALLOC_BUFFER enum
commit c307459 upstream. FIRMWARE_PREALLOC_BUFFER is a "how", not a "what", and confuses the LSMs that are interested in filtering between types of things. The "how" should be an internal detail made uninteresting to the LSMs. Fixes: a098ecd ("firmware: support loading into a pre-allocated buffer") Fixes: fd90bc5 ("ima: based on policy verify firmware signatures (pre-allocated buffer)") Fixes: 4f0496d ("ima: based on policy warn about loading firmware (pre-allocated buffer)") Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> Acked-by: Scott Branden <scott.branden@broadcom.com> Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20201002173828.2099543-2-keescook@chromium.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 7a5b764 commit e1043d8

7 files changed

Lines changed: 11 additions & 14 deletions

File tree

drivers/base/firmware_loader/main.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -470,14 +470,12 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
470470
int i, len;
471471
int rc = -ENOENT;
472472
char *path;
473-
enum kernel_read_file_id id = READING_FIRMWARE;
474473
size_t msize = INT_MAX;
475474
void *buffer = NULL;
476475

477476
/* Already populated data member means we're loading into a buffer */
478477
if (!decompress && fw_priv->data) {
479478
buffer = fw_priv->data;
480-
id = READING_FIRMWARE_PREALLOC_BUFFER;
481479
msize = fw_priv->allocated_size;
482480
}
483481

@@ -501,7 +499,8 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
501499

502500
/* load firmware files from the mount namespace of init */
503501
rc = kernel_read_file_from_path_initns(path, &buffer,
504-
&size, msize, id);
502+
&size, msize,
503+
READING_FIRMWARE);
505504
if (rc) {
506505
if (rc != -ENOENT)
507506
dev_warn(device, "loading %s failed with error %d\n",

fs/exec.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -955,6 +955,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
955955
{
956956
loff_t i_size, pos;
957957
ssize_t bytes = 0;
958+
void *allocated = NULL;
958959
int ret;
959960

960961
if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
@@ -978,8 +979,8 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
978979
goto out;
979980
}
980981

981-
if (id != READING_FIRMWARE_PREALLOC_BUFFER)
982-
*buf = vmalloc(i_size);
982+
if (!*buf)
983+
*buf = allocated = vmalloc(i_size);
983984
if (!*buf) {
984985
ret = -ENOMEM;
985986
goto out;
@@ -1008,7 +1009,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
10081009

10091010
out_free:
10101011
if (ret < 0) {
1011-
if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
1012+
if (allocated) {
10121013
vfree(*buf);
10131014
*buf = NULL;
10141015
}

include/linux/fs.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2861,7 +2861,6 @@ extern int do_pipe_flags(int *, int);
28612861
#define __kernel_read_file_id(id) \
28622862
id(UNKNOWN, unknown) \
28632863
id(FIRMWARE, firmware) \
2864-
id(FIRMWARE_PREALLOC_BUFFER, firmware) \
28652864
id(MODULE, kernel-module) \
28662865
id(KEXEC_IMAGE, kexec-image) \
28672866
id(KEXEC_INITRAMFS, kexec-initramfs) \

kernel/module.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4028,7 +4028,7 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
40284028
{
40294029
struct load_info info = { };
40304030
loff_t size;
4031-
void *hdr;
4031+
void *hdr = NULL;
40324032
int err;
40334033

40344034
err = may_init_module();

security/integrity/digsig.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ int __init integrity_add_key(const unsigned int id, const void *data,
169169

170170
int __init integrity_load_x509(const unsigned int id, const char *path)
171171
{
172-
void *data;
172+
void *data = NULL;
173173
loff_t size;
174174
int rc;
175175
key_perm_t perm;

security/integrity/ima/ima_fs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ static const struct file_operations ima_ascii_measurements_ops = {
272272

273273
static ssize_t ima_read_policy(char *path)
274274
{
275-
void *data;
275+
void *data = NULL;
276276
char *datap;
277277
loff_t size;
278278
int rc, pathlen = strlen(path);

security/integrity/ima/ima_main.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -621,19 +621,17 @@ void ima_post_path_mknod(struct dentry *dentry)
621621
int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
622622
{
623623
/*
624-
* READING_FIRMWARE_PREALLOC_BUFFER
625-
*
626624
* Do devices using pre-allocated memory run the risk of the
627625
* firmware being accessible to the device prior to the completion
628626
* of IMA's signature verification any more than when using two
629-
* buffers?
627+
* buffers? It may be desirable to include the buffer address
628+
* in this API and walk all the dma_map_single() mappings to check.
630629
*/
631630
return 0;
632631
}
633632

634633
const int read_idmap[READING_MAX_ID] = {
635634
[READING_FIRMWARE] = FIRMWARE_CHECK,
636-
[READING_FIRMWARE_PREALLOC_BUFFER] = FIRMWARE_CHECK,
637635
[READING_MODULE] = MODULE_CHECK,
638636
[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
639637
[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,

0 commit comments

Comments
 (0)