Hi Rob,
On Thu, 14 Aug 2025 at 17:17, Rob Herring robh@kernel.org wrote:
On Thu, Aug 14, 2025 at 11:51:44AM +0100, Daniel Stone wrote:
This is the main security issue, since it would allow writes a cmdstream BO which has been created but is not _the_ cmdstream BO for this job. Fixing that is pretty straightforward, but given that someone will almost certainly try to add dmabuf support to this driver, it's also probably worth a comment in the driver flags telling anyone who tries to add DRIVER_PRIME that they need to disallow export of cmdbuf BOs.
What would be the usecase for exporting BOs here?
I suppose if one wants to feed in camera data and we need to do the allocation in the ethos driver since it likely has more constraints (i.e. must be contiguous). (Whatever happened on the universal allocator or constraint solver? I haven't been paying attention for a while...)
Yeah, I guess it's just reasonably natural to allow export if you're allowing import as well.
Here's the reworked (but not yet tested) code which I think should solve all of the above issues. There was also an issue with the cleanup path that we wouldn't do a put on the last BO if there was a size error. We just need to set ejob->region_bo[ejob->region_cnt] and increment region_cnt before any checks.
Nice, thanks! That all looks good to me.
Using unchecked add/mul when calculating the sizes also made me raise an eyebrow - it might be provably safe but perhaps it's better to use all the helpers just to make sure undetected overflow can't occur.
Cheers, Daniel