[RFC/PATCHv3 v3 3/3] firmware: Support loading into a pre-allocated buffer

Stephen Boyd stephen.boyd at linaro.org
Wed May 11 15:11:01 PDT 2016


Quoting Mimi Zohar (2016-05-11 13:46:46)
> 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". 

Sure, READING_FIRMWARE_PREALLOC_BUFFER is fine.

> 
> >  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:

I don't think we care about the case of the pre-allocated buffer being
smaller than the firmware size though. In that case, 'max_size' will be
less than 'i_size' in kernel_read_file() and then we would fail that
function with -EFBIG. This should allow IMA to treat this the same as
the READING_FIRMWARE case.



More information about the linux-arm mailing list