On Mon 09 Jul 23:56 PDT 2018, Ard Biesheuvel wrote:

On 10 July 2018 at 08:51, Ard Biesheuvel [off-list ref] wrote:

quoted

On 9 July 2018 at 21:41, Mimi Zohar [off-list ref] wrote:

quoted

On Mon, 2018-07-02 at 17:30 +0200, Ard Biesheuvel wrote:

quoted

On 2 July 2018 at 16:38, Mimi Zohar [off-list ref] wrote:

[..]

quoted

So to summarize again: in my opinion, using a single buffer is not a
problem as long as the validation completes before the DMA map is
performed. This will provide the expected guarantees on systems with
IOMMUs, and will not complicate matters on systems where there is no
point in obsessing about this anyway given that devices can access all
of memory whenever they want to.

As for the Qualcomm case: dma_alloc_coherent() is not needed here but
simply ends up being used because it was already wired up in the
qualcomm specific secure world API, which amounts to doing syscalls
into a higher privilege level than the one the kernel itself runs at.

As I said before, the dma_alloc_coherent() referred to in this
discussion holds parameters for the Trustzone call, i.e. it will hold
the address to the buffer that the firmware was loaded into - it won't
hold any data that comes from the actual firmware.

quoted

So again, reasoning about whether the secure world will look at your
data before you checked the sig is rather pointless, and adding
special cases to the IMA api to cater for this use case seems like a
waste of engineering and review effort to me.

Forgive me if I'm missing something in the implementation here, but
aren't the IMA checks done before request_firmware*() returns?

quoted

If we have to do
something to tie up this loose end, let's try switching it to the
streaming DMA api instead.

Forgot to mention: the Qualcomm case is about passing data to the CPU
running at another privilege level, so IOMMU vs !IOMMU is not a factor
here.

Further more, all scenarios we've look at so far is completely
sequential, so if the firmware request fails we won't invoke the
Trustzone operation that would consume the memory or we won't turn on
the power to the CPU that would execute the firmware.


Tbh the only case I can think of where there would be a "race condition"
here is if we have a device that is polling the last byte of a
predefined firmware memory area for the firmware loader to read some
specific data into it. In cases where the firmware request is followed
by some explicit signalling to the device (or a power on sequence) I'm
unable to see the issue discussed here.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html