Re: [PATCH v2] i3c: master: svc: bound IBI payload to the requested max_payload_len

[Frank Li]

On Wed, Jun 24, 2026 at 01:04:33PM +0800, Maoyi Xie wrote:
> svc_i3c_master_handle_ibi() reads the IBI payload from the RX FIFO into
> the IBI slot. The loop is bounded by the hardware FIFO size
> (SVC_I3C_FIFO_SIZE), not by the slot size.
>
> slot->data points into the IBI pool, which i3c_generic_ibi_alloc_pool()
> sizes at max_payload_len per slot. svc_i3c_master_request_ibi() only
> rejects a max_payload_len larger than SVC_I3C_FIFO_SIZE, so a driver can
> request a smaller one. mctp-i3c requests 1. Each readsb() then copies the
> controller RXCOUNT bytes (up to 31) with no check against the slot size.
> A device that sends more bytes than the slot holds writes past
> slot->data, an out-of-bounds write into the IBI pool.
>
> Bound the loop by dev->ibi->max_payload_len and clamp each read to the
> space left in the slot, the same way dw-i3c does. A device can still send
> more than the requested payload. Flush the leftover bytes from the RX FIFO
> so they do not leak into the next transfer.
>
> Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Co-developed-by: Kaixuan Li <kaixuan.li@xxxxxxxxxx>
> Signed-off-by: Kaixuan Li <kaixuan.li@xxxxxxxxxx>
> Signed-off-by: Maoyi Xie <maoyixie.tju@xxxxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

> v2:
> - use min() instead of min_t(), the types already match (Frank Li)
> - flush the leftover RX FIFO bytes after the bounded read, so an
>   oversized IBI does not desync the next transfer (Sashiko AI review)
>
>  drivers/i3c/master/svc-i3c-master.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index e2d99a3ac07d..4eb54f9ee2cd 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -465,14 +465,22 @@ static int svc_i3c_master_handle_ibi(struct svc_i3c_master *master,
>  	buf = slot->data;
>
>  	while (SVC_I3C_MSTATUS_RXPEND(readl(master->regs + SVC_I3C_MSTATUS))  &&
> -	       slot->len < SVC_I3C_FIFO_SIZE) {
> +	       slot->len < dev->ibi->max_payload_len) {
>  		mdatactrl = readl(master->regs + SVC_I3C_MDATACTRL);
>  		count = SVC_I3C_MDATACTRL_RXCOUNT(mdatactrl);
> +		count = min(count, dev->ibi->max_payload_len - slot->len);
>  		readsb(master->regs + SVC_I3C_MRDATAB, buf, count);
>  		slot->len += count;
>  		buf += count;
>  	}
>
> +	/*
> +	 * The device may have sent more than the requested payload. Drop the
> +	 * extra bytes so they do not leak into the next transfer.
> +	 */
> +	if (SVC_I3C_MSTATUS_RXPEND(readl(master->regs + SVC_I3C_MSTATUS)))
> +		writel(SVC_I3C_MDATACTRL_FLUSHRB, master->regs + SVC_I3C_MDATACTRL);
> +
>  	master->ibi.tbq_slot = slot;
>
>  	return 0;