Re: [PATCH v4 1/5] accel/amdxdna: refuse an I/O memory mapping of an imported BO

On 8/17/26 16:07, Taimuraz Kaitmazov wrote:
> amdxdna_gem_vmap() flattens the iosys_map drm_gem_vmap() fills in down to
> the void * in abo->mem.kva, and iosys_map is discriminated by is_iomem, so
> an exporter answering with an I/O mapping leaves a void __iomem pointer
> there, which amdxdna_cmd_set_error() memsets and memcpys through.
>
> amdxdna_drm_va_tbl takes a dmabuf_fd, so such a BO can be any exporter's
> buffer. amdgpu cannot reach this: its .pin forces GTT for a non peer to
> peer attachment like ours. An exporter on drm_gem_prime_dmabuf_ops has
> no .pin, and drm_gem_ttm_vmap() answers iomem for a VRAM resident
> object, so an NPU paired with nouveau or radeon does.
>
> Drop such a mapping and answer NULL. Checking here rather than in the
> .vmap callback leaves that callback's iosys_map contract intact for a
> caller equipped to read I/O memory, and covers everything that takes a
> plain kernel address through this helper. vmw_gem_vmap() refuses the
> same case; unlike that one this path is reachable from an unprivileged
> ioctl, so it neither warns nor logs at error level.
>
> Signed-off-by: Taimuraz Kaitmazov <[email protected]>
> ---
>   drivers/accel/amdxdna/amdxdna_gem.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index cca84fa07e9d..f88b5349cd4b 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -209,10 +209,15 @@ void *amdxdna_gem_vmap(struct amdxdna_gem_obj *abo)
>   
>   	if (!abo->mem.kva) {
>   		ret = drm_gem_vmap(to_gobj(abo), &map);
> -		if (ret)
> +		if (ret) {
>   			XDNA_ERR(abo->client->xdna, "Vmap bo failed, ret %d", ret);
> -		else
> +		} else if (map.is_iomem) {
> +			/* Callers use the result as an ordinary kernel address. */
> +			XDNA_DBG(abo->client->xdna, "Vmap bo returned I/O memory");
> +			drm_gem_vunmap(to_gobj(abo), &map);
> +		} else {
>   			abo->mem.kva = map.vaddr;
> +		}
Reviewed-by: Lizhi Hou <[email protected]>
>   	}
>   	return abo->mem.kva;
>   }