Re: [PATCH v2] accel/amdxdna: fix race condition in mailbox send path

[Lizhi Hou]

On 8/29/26 00:53, Deniz Aydogan wrote:
> mailbox_send_msg() reads and updates x2i_tail without any
> synchronization. The DRM scheduler and ioctl paths can call
> xdna_mailbox_send_msg() concurrently on the same channel, which
> allows two threads to read the same tail value and corrupt the
> hardware ring buffer.

Is this trying to address the sashiko comment?

The config cu ioctl should not concurrently running with commands 
(submitting by DRM scheduler). If this happens, it indicates a bug in 
user space. And the commands submitted to the same hwctx (mailbox 
channel) could fail. The incorrect application may mess up itself and it 
would not impact other processes.

Thanks,

Lizhi

> Add a mutex to serialize the entire send path. A mutex is used
> rather than a spinlock because the existing code calls
> read_poll_timeout() with a non-zero sleep, which can reschedule.
>
> Fixes: 3ba13f5e7180 ("Merge tag 'devicetree-fixes-for-7.3-1'")
> Signed-off-by: Deniz Aydogan <denizaydogan1902@xxxxxxxxx>
> ---
>   drivers/accel/amdxdna/amdxdna_mailbox.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.c b/drivers/accel/amdxdna/amdxdna_mailbox.c
> index 271617347..8338f03bd 100644
> --- a/drivers/accel/amdxdna/amdxdna_mailbox.c
> +++ b/drivers/accel/amdxdna/amdxdna_mailbox.c
> @@ -8,6 +8,7 @@
>   #include <linux/bitfield.h>
>   #include <linux/interrupt.h>
>   #include <linux/iopoll.h>
> +#include <linux/mutex.h>
>   #include <linux/slab.h>
>   #include <linux/xarray.h>
>   
> @@ -60,6 +61,7 @@ struct mailbox_channel {
>   	struct xarray			chan_xa;
>   	u32				next_msgid;
>   	u32				x2i_tail;
> +	struct mutex			lock;
>   
>   	/* Received msg related fields */
>   	struct workqueue_struct		*work_q;
> @@ -205,6 +207,7 @@ mailbox_send_msg(struct mailbox_channel *mb_chann, struct mailbox_msg *mb_msg)
>   	u32 tmp_tail;
>   	int ret;
>   
> +	mutex_lock(&mb_chann->lock);
>   	head = mailbox_get_headptr(mb_chann, CHAN_RES_X2I);
>   	tail = mb_chann->x2i_tail;
>   	ringbuf_size = mailbox_get_ringbuf_size(mb_chann, CHAN_RES_X2I) - sizeof(u32);
> @@ -225,8 +228,10 @@ mailbox_send_msg(struct mailbox_channel *mb_chann, struct mailbox_msg *mb_msg)
>   		ret = read_poll_timeout(mailbox_get_headptr, head,
>   					tmp_tail < head || tail >= head,
>   					1, 100, false, mb_chann, CHAN_RES_X2I);
> -		if (ret)
> +		if (ret) {
> +			mutex_unlock(&mb_chann->lock);
>   			return ret;
> +		}
>   
>   		if (tail >= head)
>   			goto check_again;
> @@ -240,6 +245,7 @@ mailbox_send_msg(struct mailbox_channel *mb_chann, struct mailbox_msg *mb_msg)
>   			    mb_msg->pkg.header.opcode,
>   			    mb_msg->pkg.header.id);
>   
> +	mutex_unlock(&mb_chann->lock);
>   	return 0;
>   }
>   
> @@ -487,6 +493,7 @@ struct mailbox_channel *xdna_mailbox_alloc_channel(struct mailbox *mb)
>   		goto free_chann;
>   	}
>   	mb_chann->mb = mb;
> +	mutex_init(&mb_chann->lock);
>   
>   	return mb_chann;
>