Re: [PATCH] accel/amdxdna: reject a command chain that carries no commands

Re: [PATCH] accel/amdxdna: reject a command chain that carries no commands

[Lizhi Hou]

On 8/17/26 17:00, Taimuraz Kaitmazov wrote:
> A chain whose command_count is zero passes the payload length check,
> because struct_size(payload, data, 0) is just the header. The fill loop
> then does not run, so offset stays zero and the request is submitted with
> a zero-length buffer.
>
> On firmware without AIE2_NPU_COMMAND that ends at the opcode check, since
> op is still ERT_INVALID_CMD and aie2_get_chain_msg_op() answers
> MSG_OP_MAX_OPCODE. aie2_get_npu_chain_msg_op() answers
> MSG_OP_CHAIN_EXEC_NPU whatever it is given, so there the submission
> continues to drm_clflush_virt_range(cmd_buf, 0), which reads the byte
> before the buffer and faults on the vmap guard page. EXEC_CMD is
> reachable by any process that can open the render node.
>
> Reject the request instead.
>
> Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
> ---
> drm_clflush_virt_range() faulting on an empty range is a core problem, and a
> patch for it is on the list separately. This rejects the request in the driver
> regardless, since a chain carrying no commands is not something to submit.
>
>   drivers/accel/amdxdna/aie2_message.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c
> index dfe0fbdf066d..b4c49259a1a2 100644
> --- a/drivers/accel/amdxdna/aie2_message.c
> +++ b/drivers/accel/amdxdna/aie2_message.c
> @@ -994,7 +994,7 @@ int aie2_cmdlist_multi_execbuf(struct amdxdna_hwctx *hwctx,
>   	}
>   
>   	ccnt = payload->command_count;
> -	if (payload_len < struct_size(payload, data, ccnt)) {
> +	if (!ccnt || payload_len < struct_size(payload, data, ccnt)) {

Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>

I will add a Fixes tag when I merge it.

Thanks,

Lizhi

>   		XDNA_DBG(xdna, "Invalid command count %d", ccnt);
>   		return -EINVAL;
>   	}

Re: [PATCH] accel/amdxdna: reject a command chain that carries no commands

[Lizhi Hou]

Applied to drm-misc-fixes

On 8/21/26 10:46, Lizhi Hou wrote:
>
> On 8/17/26 17:00, Taimuraz Kaitmazov wrote:
>> A chain whose command_count is zero passes the payload length check,
>> because struct_size(payload, data, 0) is just the header. The fill loop
>> then does not run, so offset stays zero and the request is submitted 
>> with
>> a zero-length buffer.
>>
>> On firmware without AIE2_NPU_COMMAND that ends at the opcode check, 
>> since
>> op is still ERT_INVALID_CMD and aie2_get_chain_msg_op() answers
>> MSG_OP_MAX_OPCODE. aie2_get_npu_chain_msg_op() answers
>> MSG_OP_CHAIN_EXEC_NPU whatever it is given, so there the submission
>> continues to drm_clflush_virt_range(cmd_buf, 0), which reads the byte
>> before the buffer and faults on the vmap guard page. EXEC_CMD is
>> reachable by any process that can open the render node.
>>
>> Reject the request instead.
>>
>> Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
>> ---
>> drm_clflush_virt_range() faulting on an empty range is a core 
>> problem, and a
>> patch for it is on the list separately. This rejects the request in 
>> the driver
>> regardless, since a chain carrying no commands is not something to 
>> submit.
>>
>>   drivers/accel/amdxdna/aie2_message.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/accel/amdxdna/aie2_message.c 
>> b/drivers/accel/amdxdna/aie2_message.c
>> index dfe0fbdf066d..b4c49259a1a2 100644
>> --- a/drivers/accel/amdxdna/aie2_message.c
>> +++ b/drivers/accel/amdxdna/aie2_message.c
>> @@ -994,7 +994,7 @@ int aie2_cmdlist_multi_execbuf(struct 
>> amdxdna_hwctx *hwctx,
>>       }
>>         ccnt = payload->command_count;
>> -    if (payload_len < struct_size(payload, data, ccnt)) {
>> +    if (!ccnt || payload_len < struct_size(payload, data, ccnt)) {
>
> Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>
> I will add a Fixes tag when I merge it.
>
> Thanks,
>
> Lizhi
>
>>           XDNA_DBG(xdna, "Invalid command count %d", ccnt);
>>           return -EINVAL;
>>       }