On Wed, 2016-05-11 at 13:22 -0700, Stephen Boyd wrote: > Quoting Mimi Zohar (2016-05-11 05:41:52) > > Hi Stephen, > > > > On Tue, 2016-05-10 at 15:26 -0700, Stephen Boyd wrote: > > > > > -static int fw_get_filesystem_firmware(struct device *device, > > > - struct firmware_buf *buf) > > > +static int > > > +fw_get_filesystem_firmware(struct device *device, struct firmware_buf > > > *buf) > > > { > > > loff_t size; > > > int i, len; > > > int rc = -ENOENT; > > > char *path; > > > + enum kernel_read_file_id id = READING_FIRMWARE; > > > + size_t msize = INT_MAX; > > > + > > > + /* Already populated data member means we're loading into a buffer > > > */ > > > + if (buf->data) { > > > + id = READING_FIRMWARE_INTO_BUF; > > > > In both cases we're reading the firmware into a buffer. In this case, > > it is pre-allocated. Other than it being pre-allocated, is there > > anything special about this buffer? > > No. I'm not sure what you're asking/implying. > > > There has to be a more appropriate > > string identifier. > > Ok. Any suggestions? The point of the new id is so that we know if we > need to allocate the buffer or not in kernel_read_file().
If you're still using DMA, then perhaps "READING_FIRMWARE_DMA". If the only difference is that the buffer is pre-allocated, then maybe "READING_FIRMWARE_PREALLOC_BUFFER". > Alternatively > we could indicate that by a NULL *buf pointer, but it seems that half > the time that's not initialized to NULL so it didn't seem safe to rely > on that fact or update the callsites appropriately. Assuming that the pre-allocated buffer is smaller than the firmware size, then a new name definitely needs to be specified, so that the firmware signature can be verified on the security_kernel_read_file() hook, as opposed to the security_kernel_post_read_file() hook. The patch would look something like: diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 68b26c3..c799459 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -359,6 +359,10 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id) #endif return 0; /* We rely on module signature checking */ } + + if (file && read_id == READING_FIRMWARE_INTO_BUF) + return process_measurement(file, NULL, 0, MAY_READ, + FIRMWARE_CHECK, 0); return 0; } @@ -404,6 +408,9 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, return 0; } + if (file && read_id == READING_FIRMWARE_INTO_BUF) + return 0; + func = read_idmap[read_id] ?: FILE_CHECK; return process_measurement(file, buf, size, MAY_READ, func, 0); } Once we've decided on a more appropriate identifier string, I'll post the patch. Mimi