* [PATCH V2] accel/amdxdna: Fix potential NULL pointer dereference of abo->client
@ 2026-07-07 20:15 Lizhi Hou
2026-07-07 21:38 ` Max Zhen
0 siblings, 1 reply; 2+ messages in thread
From: Lizhi Hou @ 2026-07-07 20:15 UTC (permalink / raw)
To: ogabbay, quic_jhugo, dri-devel, mario.limonciello, karol.wachowski
Cc: Lizhi Hou, linux-kernel, max.zhen, sonal.santan
Closing a BO handle clears abo->client, while the underlying GEM object
may remain alive due to internal kernel references. As a result, code
executed after the BO handle is closed may dereference a NULL abo->client
pointer.
Remove accesses to abo->client from code paths that may execute after the
BO handle has been closed.
Fixes: d76856beb4a4 ("accel/amdxdna: Refactor GEM BO handling and add helper APIs for address retrieval")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
drivers/accel/amdxdna/aie2_message.c | 4 ++--
drivers/accel/amdxdna/amdxdna_gem.c | 11 +++++++++--
drivers/accel/amdxdna/amdxdna_gem.h | 11 +++++++++--
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c
index c4b364801cc0..dfe0fbdf066d 100644
--- a/drivers/accel/amdxdna/aie2_message.c
+++ b/drivers/accel/amdxdna/aie2_message.c
@@ -840,7 +840,7 @@ static struct aie2_exec_msg_ops npu_exec_message_ops = {
static int aie2_init_exec_req(void *req, struct amdxdna_gem_obj *cmd_abo,
size_t *size, u32 *msg_op)
{
- struct amdxdna_dev *xdna = cmd_abo->client->xdna;
+ struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(cmd_abo)->dev);
int ret;
u32 op;
@@ -874,7 +874,7 @@ static int
aie2_cmdlist_fill_slot(void *slot, struct amdxdna_gem_obj *cmd_abo,
size_t *size, u32 *cmd_op)
{
- struct amdxdna_dev *xdna = cmd_abo->client->xdna;
+ struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(cmd_abo)->dev);
int ret;
u32 op;
diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index 1275f91ca705..4628a2787265 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -198,6 +198,7 @@ amdxdna_gem_destroy_obj(struct amdxdna_gem_obj *abo)
*/
void *amdxdna_gem_vmap(struct amdxdna_gem_obj *abo)
{
+ struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
struct iosys_map map = IOSYS_MAP_INIT_VADDR(NULL);
int ret;
@@ -210,7 +211,7 @@ void *amdxdna_gem_vmap(struct amdxdna_gem_obj *abo)
if (!abo->mem.kva) {
ret = drm_gem_vmap(to_gobj(abo), &map);
if (ret)
- XDNA_ERR(abo->client->xdna, "Vmap bo failed, ret %d", ret);
+ XDNA_ERR(xdna, "Vmap bo failed, ret %d", ret);
else
abo->mem.kva = map.vaddr;
}
@@ -354,7 +355,13 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo,
unsigned long nr_pages;
int ret;
- if (!amdxdna_pasid_on(abo->client)) {
+ /*
+ * When PASID is off, amdxdna_gem_obj_open() called amdxdna_dma_map_bo()
+ * and mem.dma_addr is valid; use the DMA address directly and skip HMM.
+ * Avoid dereferencing abo->client which may be NULL (cleared in close())
+ * while internal kernel references are still held.
+ */
+ if (abo->mem.dma_addr != AMDXDNA_INVALID_ADDR) {
/* Need to set uva for heap uva validation */
abo->mem.uva = addr;
return 0;
diff --git a/drivers/accel/amdxdna/amdxdna_gem.h b/drivers/accel/amdxdna/amdxdna_gem.h
index a35d2f15d32c..1e90e32bf3cd 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.h
+++ b/drivers/accel/amdxdna/amdxdna_gem.h
@@ -88,12 +88,19 @@ u64 amdxdna_gem_dev_addr(struct amdxdna_gem_obj *abo);
static inline u64 amdxdna_dev_bo_offset(struct amdxdna_gem_obj *abo)
{
- return amdxdna_gem_dev_addr(abo) - abo->client->xdna->dev_info->dev_mem_base;
+ return amdxdna_gem_dev_addr(abo) - to_xdna_dev(to_gobj(abo)->dev)->dev_info->dev_mem_base;
}
static inline u64 amdxdna_obj_dma_addr(struct amdxdna_gem_obj *abo)
{
- return amdxdna_pasid_on(abo->client) ? amdxdna_gem_uva(abo) : abo->mem.dma_addr;
+ /*
+ * amdxdna_gem_obj_open() calls amdxdna_dma_map_bo() only when PASID is
+ * off, leaving mem.dma_addr at AMDXDNA_INVALID_ADDR when PASID is on.
+ * Avoid dereferencing abo->client, which is cleared to NULL by
+ * amdxdna_gem_obj_close() while internal kernel references remain.
+ */
+ return (abo->mem.dma_addr != AMDXDNA_INVALID_ADDR) ?
+ abo->mem.dma_addr : amdxdna_gem_uva(abo);
}
void amdxdna_umap_put(struct amdxdna_umap *mapp);
--
2.34.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH V2] accel/amdxdna: Fix potential NULL pointer dereference of abo->client
2026-07-07 20:15 [PATCH V2] accel/amdxdna: Fix potential NULL pointer dereference of abo->client Lizhi Hou
@ 2026-07-07 21:38 ` Max Zhen
0 siblings, 0 replies; 2+ messages in thread
From: Max Zhen @ 2026-07-07 21:38 UTC (permalink / raw)
To: Lizhi Hou, ogabbay, quic_jhugo, dri-devel, mario.limonciello,
karol.wachowski
Cc: linux-kernel, sonal.santan
On 7/7/2026 Tue 13:15, Lizhi Hou wrote:
> Closing a BO handle clears abo->client, while the underlying GEM object
> may remain alive due to internal kernel references. As a result, code
> executed after the BO handle is closed may dereference a NULL abo->client
> pointer.
>
> Remove accesses to abo->client from code paths that may execute after the
> BO handle has been closed.
>
> Fixes: d76856beb4a4 ("accel/amdxdna: Refactor GEM BO handling and add helper APIs for address retrieval")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Max Zhen <max.zhen@amd.com>
> ---
> drivers/accel/amdxdna/aie2_message.c | 4 ++--
> drivers/accel/amdxdna/amdxdna_gem.c | 11 +++++++++--
> drivers/accel/amdxdna/amdxdna_gem.h | 11 +++++++++--
> 3 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c
> index c4b364801cc0..dfe0fbdf066d 100644
> --- a/drivers/accel/amdxdna/aie2_message.c
> +++ b/drivers/accel/amdxdna/aie2_message.c
> @@ -840,7 +840,7 @@ static struct aie2_exec_msg_ops npu_exec_message_ops = {
> static int aie2_init_exec_req(void *req, struct amdxdna_gem_obj *cmd_abo,
> size_t *size, u32 *msg_op)
> {
> - struct amdxdna_dev *xdna = cmd_abo->client->xdna;
> + struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(cmd_abo)->dev);
> int ret;
> u32 op;
>
> @@ -874,7 +874,7 @@ static int
> aie2_cmdlist_fill_slot(void *slot, struct amdxdna_gem_obj *cmd_abo,
> size_t *size, u32 *cmd_op)
> {
> - struct amdxdna_dev *xdna = cmd_abo->client->xdna;
> + struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(cmd_abo)->dev);
> int ret;
> u32 op;
>
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 1275f91ca705..4628a2787265 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -198,6 +198,7 @@ amdxdna_gem_destroy_obj(struct amdxdna_gem_obj *abo)
> */
> void *amdxdna_gem_vmap(struct amdxdna_gem_obj *abo)
> {
> + struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
> struct iosys_map map = IOSYS_MAP_INIT_VADDR(NULL);
> int ret;
>
> @@ -210,7 +211,7 @@ void *amdxdna_gem_vmap(struct amdxdna_gem_obj *abo)
> if (!abo->mem.kva) {
> ret = drm_gem_vmap(to_gobj(abo), &map);
> if (ret)
> - XDNA_ERR(abo->client->xdna, "Vmap bo failed, ret %d", ret);
> + XDNA_ERR(xdna, "Vmap bo failed, ret %d", ret);
> else
> abo->mem.kva = map.vaddr;
> }
> @@ -354,7 +355,13 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo,
> unsigned long nr_pages;
> int ret;
>
> - if (!amdxdna_pasid_on(abo->client)) {
> + /*
> + * When PASID is off, amdxdna_gem_obj_open() called amdxdna_dma_map_bo()
> + * and mem.dma_addr is valid; use the DMA address directly and skip HMM.
> + * Avoid dereferencing abo->client which may be NULL (cleared in close())
> + * while internal kernel references are still held.
> + */
> + if (abo->mem.dma_addr != AMDXDNA_INVALID_ADDR) {
> /* Need to set uva for heap uva validation */
> abo->mem.uva = addr;
> return 0;
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.h b/drivers/accel/amdxdna/amdxdna_gem.h
> index a35d2f15d32c..1e90e32bf3cd 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.h
> +++ b/drivers/accel/amdxdna/amdxdna_gem.h
> @@ -88,12 +88,19 @@ u64 amdxdna_gem_dev_addr(struct amdxdna_gem_obj *abo);
>
> static inline u64 amdxdna_dev_bo_offset(struct amdxdna_gem_obj *abo)
> {
> - return amdxdna_gem_dev_addr(abo) - abo->client->xdna->dev_info->dev_mem_base;
> + return amdxdna_gem_dev_addr(abo) - to_xdna_dev(to_gobj(abo)->dev)->dev_info->dev_mem_base;
> }
>
> static inline u64 amdxdna_obj_dma_addr(struct amdxdna_gem_obj *abo)
> {
> - return amdxdna_pasid_on(abo->client) ? amdxdna_gem_uva(abo) : abo->mem.dma_addr;
> + /*
> + * amdxdna_gem_obj_open() calls amdxdna_dma_map_bo() only when PASID is
> + * off, leaving mem.dma_addr at AMDXDNA_INVALID_ADDR when PASID is on.
> + * Avoid dereferencing abo->client, which is cleared to NULL by
> + * amdxdna_gem_obj_close() while internal kernel references remain.
> + */
> + return (abo->mem.dma_addr != AMDXDNA_INVALID_ADDR) ?
> + abo->mem.dma_addr : amdxdna_gem_uva(abo);
> }
>
> void amdxdna_umap_put(struct amdxdna_umap *mapp);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-07 21:38 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-07 20:15 [PATCH V2] accel/amdxdna: Fix potential NULL pointer dereference of abo->client Lizhi Hou
2026-07-07 21:38 ` Max Zhen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome