Hi Rob, On Thu, 14 Aug 2025 at 17:17, Rob Herring <[email protected]> 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
