Re: [PATCH v4 0/5] accel/amdxdna: SYNC_BO correctness fixes

[Lizhi Hou]

On 8/18/26 14:40, Taimuraz Kaitmazov wrote:
> Patch 1 needs a prerequisite. It makes amdxdna_gem_vmap() answer NULL 
> on an iomem exporter, and the eight amdxdna_cmd_get_payload() callers 
> in aie2_message.c check neither the pointer nor the length it leaves 
> unwritten there. A command BO can be an import, so patch 1 alone turns 
> a silent __iomem write into a NULL deref with an uninitialised length.
> https://lore.kernel.org/all/20260818002459.377641-1-taimuraz@xxxxxxxxxxxxx/ 

amdxdna_cmd_get_op() is always called before amdxdna_cmd_get_payload() 
for the same BO, and since amdxdna_gem_vmap() caches its results, the 
vmap inside get_payload() is currently guaranteed to succeed. The patch 
is therefore defensive rather than fixing a currently reachable crash.

Lizhi

> fixes the callers. Happy to respin on top if you prefer them together.
>
> Taimuraz
>
> On 8/18/26 02:07, Taimuraz Kaitmazov wrote:
> > Five fixes in and around amdxdna_drm_sync_bo_ioctl().
> >
> > Patch 1 refuses an I/O memory mapping the driver would otherwise 
> > store as
> > if it were an ordinary kernel address. Patch 2 checks the device-BO 
> > range
> > for overflow. Patch 4 refuses a flush of an imported BO, which is why
> > patch 3 comes first: the ioctl answers a rejected flush with drm_WARN(),
> > so without it an ordinary sync on an imported BO splats. Patch 5 stops
> > the ioctl reporting failure for a flush that succeeded.
> >
> > v3's zero-length patch has left this series. On hardware it turns out to
> > be a page fault in drm_clflush_virt_range() rather than the tidy-up its
> > commit message described, so it is a -fixes patch now, sent 
> > separately as
> > "accel/amdxdna: return early from a zero-length flush" with Fixes: and
> > Cc: stable. Patch 4 here needs its hunk, so this series wants that one
> > first.
> >
> > Changes in v4:
> >    - patch 1: the is_iomem check moved from the .vmap callback into
> >      amdxdna_gem_vmap(), per Lizhi, and it logs at debug level rather 
> > than
> >      error, since an unprivileged caller can repeat it.
> >    - new patch 3: an unprivileged SYNC_BO with an out-of-range offset
> >      already reaches that drm_WARN() today. Sashiko's review of v3 3/3
> >      flagged the same thing.
> >    - new patch 4: refuses the flush for every imported BO, as asked.
> >    - new patch 5: the debug-BO sync I mentioned on the v2 thread. 
> > Only a BO
> >      attached with ATTACH_DEBUG_BO has an assigned hwctx, so every other
> >      FROM_DEVICE sync ends in -EINVAL with its flush already done. The
> >      -EINVAL reproduces on a Strix Point NPU.
> >
> > On patch 4, one consequence worth deciding before it lands. A heap BO is
> > created through amdxdna_drm_create_share_bo(), so a device running on
> > carveout memory reaches its heap through a cbuf, is_import_bo() is 
> > true of
> > it, and SYNC_BO on every AMDXDNA_BO_DEV now answers -EOPNOTSUPP. Today
> > that path flushes nothing anyway -- amdxdna_cbuf_map() fills in only the
> > DMA address and length, so drm_clflush_sg() walks zero pages -- so the
> > change is silent no-op to hard error, and XRT's dbg_buffer::sync() 
> > reaches
> > it without Debug.force_driver_sync. If you would rather keep our own
> > exporters working, I have the variant keyed on the exporter's ops, which
> > confines the refusal to foreign buffers. Say which you prefer.
> >
> > v3: 
> > https://lore.kernel.org/all/20260813164700.43960-1-taimuraz@xxxxxxxxxxxxx/
> >
> > Built on drm-misc-next, each commit on its own: x86_64 with
> > DRM_ACCEL_AMDXDNA=m, clang 22.1.8, no warnings. Patch 5's reproducer was
> > run against 7.1.8's in-tree driver, where that call is unchanged.
> >
> > Taimuraz Kaitmazov (5):
> >    accel/amdxdna: refuse an I/O memory mapping of an imported BO
> >    accel/amdxdna: check the sync range for overflow on a device BO
> >    accel/amdxdna: do not warn when a sync request is rejected
> >    accel/amdxdna: refuse to flush an imported BO
> >    accel/amdxdna: do not fail a sync for a BO with no debug context
> >
> >   drivers/accel/amdxdna/amdxdna_ctx.c |  3 ++-
> >   drivers/accel/amdxdna/amdxdna_gem.c | 27 +++++++++++++++++++--------
> >   2 files changed, 21 insertions(+), 9 deletions(-)