On Thu, May 3, 2018 at 5:21 PM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote: > Android folks, poke below. otherwise we'll have no option but to seriously > consider Mimi's patch to prevent these calls when IMA appraisal is enforced: Sorry, figuring out who's the right person to answer this, will get back to you ASAP. Martijn > > http://lkml.kernel.org/r/1525182503-13849-7-git-send-email-zohar@xxxxxxxxxxxxxxxxxx > > Please read below.... > > On Wed, Apr 25, 2018 at 05:55:57PM +0000, Luis R. Rodriguez wrote: >> On Wed, Apr 25, 2018 at 01:00:09AM -0400, 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: >> > > If its of any help -- >> > > >> > > drivers/soc/qcom/mdt_loader.c is the only driver currently using >> > > request_firmware_into_buf() however I'll note qcom_mdt_load() is used in many >> > > other drivers so they are wrappers around request_firmware_into_buf(): >> > > >> > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c: * adreno_request_fw() handles this, but qcom_mdt_load() does >> > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c: ret = qcom_mdt_load(dev, fw, fwname, GPU_PAS_ID, >> > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c: ret = qcom_mdt_load(dev, fw, newname, GPU_PAS_ID, >> > > drivers/media/platform/qcom/venus/firmware.c: ret = qcom_mdt_load(dev, mdt, fwname, VENUS_PAS_ID, mem_va, mem_phys, >> > > drivers/remoteproc/qcom_adsp_pil.c: return qcom_mdt_load(adsp->dev, fw, rproc->firmware, adsp->pas_id, >> > > drivers/remoteproc/qcom_wcnss.c: return qcom_mdt_load(wcnss->dev, fw, rproc->firmware, WCNSS_PAS_ID, >> > > >> > > > > 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. > > To be very clear I believe Stephen implied this was not DMA buffer. Mimi > asked for READING_FIRMWARE_DMA if it was: > > https://patchwork.kernel.org/patch/9074611/ > >> > The call sequence: >> > qcom_mdt_load() -> qcom_scm_pas_init_image() -> dma_alloc_coherent() >> > >> > If dma_alloc_coherent() isn't allocating a DMA buffer, then the >> > function name is misleading/confusing. >> >> Hah, by *definition* the device *and* processor has immediate access >> to data written *immediately* when dma_alloc_coherent() is used. From >> Documentation/DMA-API.txt: >> >> ----------------------------------------------------------------------- >> Part Ia - Using large DMA-coherent buffers >> ------------------------------------------ >> >> :: >> >> void * >> dma_alloc_coherent(struct device *dev, size_t size, >> dma_addr_t *dma_handle, gfp_t flag) >> >> Consistent memory is memory for which a write by either the device or >> the processor can immediately be read by the processor or device >> without having to worry about caching effects. (You may however need >> to make sure to flush the processor's write buffers before telling >> devices to read that memory.) >> ------------------------------------------------------------------------ >> >> Is ptr below >> >> ret = request_firmware_into_buf(&seg_fw, fw_name, dev, >> ptr, phdr->p_filesz); >> >> Also part of the DMA buffer allocated earlier via: >> >> ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size); >> >> Android folks? > > Android folks? > >> > > 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? >> >> It will depend on the above question. > > Luis -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
