[PATCH] drm/amdgpu: Fix NPA-REVOKE racing an in-flight UALink import

[PATCH] drm/amdgpu: Fix NPA-REVOKE racing an in-flight UALink import

[David Carlier]

The exporter records an importer when it answers NPA-REQ, so it can send
NPA-REVOKE as soon as the BO is freed, before the importer has finished
building the dma-buf for that handle. The revoke handler assumes a fully
imported node: it dereferences imp_xa_node->dmabuf, which is still NULL
until the import completes, and drops the xarray reference the importing
thread still relies on. The importer then links the node and marks it
READY regardless, so the node can be freed while still on the per-remote
list.

When the revoke hits a node that is still NOT_READY or PENDING, only
mark it for teardown and send NPA-RELEASE, as nothing has been handed to
user-space yet. The importer checks for teardown under the xarray lock
before linking the node and marking it READY, and unwinds otherwise.

Fixes: 7cc82cd90d35 ("drm/amdgpu: Implement mechanism to revoke exported memory")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: David Carlier <devnexen@gmail.com>
---
Found by code analysis; not tested on hardware (needs two UALink-connected
accelerators in a vPod). Compile-tested with W=1. Applies on next-20260925
and does not overlap with Mukul's "UALink fixes" v2 series.
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 30 +++++++++++++++++++---
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index 8411ea17172f..90d2a525ff5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -3265,6 +3265,7 @@ static void amdgpu_ualink_process_npa_revoke_msg(struct amdgpu_device *adev,
 {
 	struct amdgpu_ualink_imp_xa_node *imp_xa_node;
 	struct amdgpu_bo *bo;
+	u32 node_state;
 	int r = 0;
 
 	/* Remove the entry from the Xarray. */
@@ -3288,7 +3289,19 @@ static void amdgpu_ualink_process_npa_revoke_msg(struct amdgpu_device *adev,
 		return;
 	}
 
+	node_state = READ_ONCE(imp_xa_node->node_state);
 	WRITE_ONCE(imp_xa_node->node_state, AMDGPU_UALINK_NODE_TEARDOWN);
+
+	/* The import is still in flight: the dmabuf may not exist yet and
+	 * nothing has been handed to user-space. Leave the node to the
+	 * importing thread, which sees the teardown state and unwinds.
+	 */
+	if (node_state == AMDGPU_UALINK_NODE_NOT_READY ||
+	    node_state == AMDGPU_UALINK_NODE_PENDING) {
+		xa_unlock(&adev->ualink.imp_xa);
+		goto send_release;
+	}
+
 	list_del_init(&imp_xa_node->list);
 	xa_unlock(&adev->ualink.imp_xa);
 
@@ -3299,6 +3312,7 @@ static void amdgpu_ualink_process_npa_revoke_msg(struct amdgpu_device *adev,
 	/* Drop the refcount for the node */
 	amdgpu_ualink_imp_xa_entry_put(imp_xa_node);
 
+send_release:
 	r = amdgpu_ualink_send_npa_release_msg(adev, remote_acc_id, handle);
 	if (r)
 		dev_err(adev->dev,
@@ -3760,9 +3774,20 @@ static int amdgpu_ualink_do_import_handle(struct amdgpu_device *adev,
 		return r;
 	}
 
-	/* Add this node to the imported handles list for the remote GPU */
+	/* Add this node to the imported handles list for the remote GPU,
+	 * unless the exporter revoked the handle while the import was in
+	 * flight. The dmabuf is released with the last node reference.
+	 */
 	xa_lock(&adev->ualink.imp_xa);
+	if (READ_ONCE(imp_xa_node->node_state) == AMDGPU_UALINK_NODE_TEARDOWN) {
+		xa_unlock(&adev->ualink.imp_xa);
+		dev_warn(adev->dev,
+			 "IMPORT: handle:%llx:%llx revoked during import\n",
+			 handle.handle_hi, handle.handle_lo);
+		return -EINVAL;
+	}
 	list_add(&imp_xa_node->list, &adev->ualink.imp_handles_list[remote_acc_id]);
+	WRITE_ONCE(imp_xa_node->node_state, AMDGPU_UALINK_NODE_READY);
 	xa_unlock(&adev->ualink.imp_xa);
 
 	return 0;
@@ -3938,9 +3963,6 @@ int amdgpu_ualink_import_handle(struct drm_device *dev,
 					"IMPORT: XA import failed for handle:%llx:%llx\n",
 					handle.handle_hi, handle.handle_lo);
 			goto cleanup;
-		} else {
-			WRITE_ONCE(imp_xa_node->node_state,
-				   AMDGPU_UALINK_NODE_READY);
 		}
 	}
 
-- 
2.55.0

Re: [PATCH] drm/amdgpu: Fix NPA-REVOKE racing an in-flight UALink import

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] amdgpu: duplicate in-flight NPA-REVOKE bypasses early exit causing NULL deref
- [Medium] amdgpu: in-flight NPA-REVOKE fails to complete npa_done causing spurious timeout
--

commit 707623cd9d98bbd57c7b50f6dc2ad0e819566c82
Author: David Carlier <devnexen@gmail.com>

drm/amdgpu: Fix NPA-REVOKE racing an in-flight UALink import

This patch modifies the NPA-REVOKE handler to cleanly mark in-flight UALink
imports for teardown rather than crashing by dereferencing a NULL dmabuf.
The importer thread now checks for this teardown state and unwinds safely
if the handle was revoked before the import completed.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -3288,7 +3289,19 @@ static void amdgpu_ualink_process_npa_revoke_msg(struct amdgpu_device *adev,
>  		return;
>  	}
>  
> +	node_state = READ_ONCE(imp_xa_node->node_state);
>  	WRITE_ONCE(imp_xa_node->node_state, AMDGPU_UALINK_NODE_TEARDOWN);
> +
> +	/* The import is still in flight: the dmabuf may not exist yet and
> +	 * nothing has been handed to user-space. Leave the node to the
> +	 * importing thread, which sees the teardown state and unwinds.
> +	 */
> +	if (node_state == AMDGPU_UALINK_NODE_NOT_READY ||
> +	    node_state == AMDGPU_UALINK_NODE_PENDING) {
> +		xa_unlock(&adev->ualink.imp_xa);
> +		goto send_release;
> +	}

[Severity: High]
If a duplicate NPA-REVOKE message arrives for an in-flight import, the
node_state is already AMDGPU_UALINK_NODE_TEARDOWN. Does this bypass the
early exit check here since it only checks for NOT_READY or PENDING?

If it bypasses this check, it will fall through to the GPUVM mappings
invalidation and unconditionally dereference imp_xa_node->dmabuf->priv
(which is NULL at this stage), causing a kernel panic.

[Severity: Medium]
When an NPA-REVOKE message arrives for an in-flight import, it sets the
node state to TEARDOWN just above, but does this strand the importing thread?

If we do not call complete(&imp_xa_node->npa_done) here, and the NPA-RSP
subsequently arrives, the RSP handler skips calling complete() because the
state is no longer NOT_READY:

amdgpu_ualink_process_npa_rsp_msg() {
    if (READ_ONCE(imp_xa_node->node_state) == AMDGPU_UALINK_NODE_NOT_READY) {
        WRITE_ONCE(imp_xa_node->node_state, AMDGPU_UALINK_NODE_PENDING);
        complete(&imp_xa_node->npa_done);
    } else {
        ...

Will this cause the importing thread to wait until the timeout expires,
resulting in a spurious timeout and an unnecessary full connection reset
of the vPod?

> +
>  	list_del_init(&imp_xa_node->list);
>  	xa_unlock(&adev->ualink.imp_xa);

[ ... ]

> @@ -3299,6 +3312,7 @@ static void amdgpu_ualink_process_npa_revoke_msg(struct amdgpu_device *adev,
>  	/* Drop the refcount for the node */
>  	amdgpu_ualink_imp_xa_entry_put(imp_xa_node);

[Severity: High]
If a duplicate revoke bypassed the early exit check above, does it also
drop the reference count a second time here?

This could lead to a double-free when the importing thread eventually cleans
up the node.

> +send_release:
>  	r = amdgpu_ualink_send_npa_release_msg(adev, remote_acc_id, handle);
>  	if (r)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260926171625.288519-1-devnexen@gmail.com?part=1