Re: [PATCH v2 2/2] accel: Add Arm Ethos-U NPU driver

[Daniel Stone]

Hi Rob,

On Thu, 14 Aug 2025 at 17:17, Rob Herring <robh@xxxxxxxxxx> 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