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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b057fb0a-5fe0-4cee-bbba-c4c88521bb86@amd.com>
Date: Fri, 15 Aug 2025 13:46:54 +0200
From: Christian König <christian.koenig@....com>
To: Daniel Stone <daniel@...ishbar.org>, Rob Herring <robh@...nel.org>
Cc: Tomeu Vizoso <tomeu@...euvizoso.net>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Oded Gabbay <ogabbay@...nel.org>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Sumit Semwal <sumit.semwal@...aro.org>, Robin Murphy <robin.murphy@....com>,
 Steven Price <steven.price@....com>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 linux-media@...r.kernel.org, linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v2 2/2] accel: Add Arm Ethos-U NPU driver

On 15.08.25 13:15, Daniel Stone wrote:
> Hi Rob,
> 
> On Thu, 14 Aug 2025 at 17:17, Rob Herring <robh@...nel.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.

I only partially agree, allowing export only makes sense if you have device memory which is not already covered by DMA-buf heaps.

So if you have special on chip memory which is accessible by for example a PCIe BAR then go ahead and provide an exporter for that.

But if your HW only needs CMA system memory then it is most likely better to use DMA-buf heaps instead and only provide an import functionality.

You can of course directly use the CMA code from your driver as well if you still want to support standard DRM, V4L or accel interfaces.

Regards,
Christian.

> 
>> 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