Re: [PATCH V1] accel/amdxdna: eliminate GFP_KERNEL allocation from mailbox send path

[Max Zhen <max.zhen@amd.com>]

On 9/30/2026 Wed 14:37, Eva Crystal wrote:
> Hi Max,
> 
>> +#define MSG_BUF_SZ(chann)                                    \
>> +     (mailbox_get_ringbuf_size(chann, CHAN_RES_X2I) +        \
>> +      sizeof(struct mailbox_msg))
> 
>> +     mb_chann->msg_buf = kcalloc(n_msg, MSG_BUF_SZ(mb_chann), GFP_KERNEL);
> 
> rb_size comes from the firmware-provided channel info, and with this
> change it now sizes a host allocation as well as the ring itself.
> Nothing bounds it before the kcalloc(), so a bad or compromised
> firmware value turns directly into an arbitrarily large kernel
> allocation. Could rb_size be checked against the ring window it
> describes before the pool is allocated?

The firmware is authenticated and signed by AMD. By design, driver 
should trust this data returned from firmware. There is no need to 
perform this check.
> 
>> +     if (!mb_chann->msg_buf)
>> +             return -ENOMEM;
> 
> This adds another failure path out of xdna_mailbox_start_channel()
> before request_irq(). On failure the caller still holds the freed
> channel pointer, which "accel/amdxdna: clear the mailbox channel
> pointer when starting it fails" (Reviewed-by: Lizhi Hou) fixes, so the
> two patches should be fine in either order.

The patch (accel/amdxdna: clear the mailbox channel
 > pointer when starting it fails) has already been applied to 
drm-misc-next (see 
https://lore.kernel.org/dri-devel/81bcf2ab-cfcd-3e2a-ca91-671e72d93bb8@amd.com/). 
My patch is based on the applied patch, so there should be no order issue.

Thanks,
Max

> 
> Thanks,
> 
> Eva Crystal (0xiviel)
> XSource Security
> https://xsourcesec.com