On Tue 24 Apr 22:00 PDT 2018, Mimi Zohar wrote: > On Tue, 2018-04-24 at 23:42 +0000, Luis R. Rodriguez wrote: > > On Tue, Apr 24, 2018 at 12:07:01PM -0400, Mimi Zohar wrote: > > > On Tue, 2018-04-24 at 17:09 +0200, Hans de Goede wrote: [..] > > > > As such the current IMA code (from v4.17-rc2) actually does > > > > not handle READING_FIRMWARE_PREALLOC_BUFFER at all, > > > > > > Right, it doesn't yet address READING_FIRMWARE_PREALLOC_BUFFER, but > > > should. > > > > > > Depending on whether the device requesting the firmware has access to > > > the DMA memory, before the signature verification, > > > > It would seem from the original patch review about READING_FIRMWARE_PREALLOC_BUFFER > > that this is not a DMA buffer. > > The call sequence: > qcom_mdt_load() -> qcom_scm_pas_init_image() -> dma_alloc_coherent() > qcom_mdt_load() is passed a struct firmware object, which "data" is passed into qcom_scm_pas_init_image(), which uses dma_alloc_coherent() to allocate scratch memory to perform a call into secure world. So the dma_alloc_coherent() here has nothing to do with firmware loading. qcom_mdt_load() will then use request_firmware_into_buf() to load other files into the passed void *mem_region, which either comes from a memremap() or dma_alloc_coherent() call. > If dma_alloc_coherent() isn't allocating a DMA buffer, then the > function name is misleading/confusing. > It does allocate memory. > > > > The device driver should have access to the buffer pointer with write given > > that with request_firmware_into_buf() the driver is giving full write access to > > the memory pointer so that the firmware API can stuff the firmware it finds > > there. > > > > Firmware signature verification would be up to the device hardware to do upon > > load *after* request_firmware_into_buf(). > > We're discussing the kernel's signature verification, not the device > hardware's signature verification. Can the device driver access the > buffer, before IMA-appraisal has verified the firmware's signature? > I would expect that it's possible to read the content of the buffer from a secondary execution context before the request_firmware_into_buf() has verified the content of the buffer, but I would be considered a seriously broken driver. Regards, Bjorn
