Re: [PATCH v4 1/3] i3c: master: dw: Report actual GET CCC payload length on success

[email protected]
Newsgroups org.infradead.lists.linux-i3c,dev.linux.lists.imx
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A pre-existing bug allows a NULL pointer dereference in dw_i3c_master_end_xfer_locked() if a SET CCC command or write transfer returns unread data without an error flag.
--

commit 6edcc13942ab9dc3576c652f10a4cea8ca077f26
Author: Adrian Ng Ho Yin <[email protected]>

i3c: master: dw: Report actual GET CCC payload length on success

This commit updates successful GET CCC transfers to report the actual
number of bytes read by setting dests[0].payload.actual_len from
RESPONSE_PORT_DATA_LEN. It also updates struct i3c_ccc_cmd_payload to
store this actual length alongside the requested buffer size.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 2f8c0c4683e06..e659ede79fc03 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -742,7 +742,10 @@ static int dw_i3c_ccc_set(struct dw_i3c_master *master,
>  		dw_i3c_master_dequeue_xfer(master, xfer);
>  
>  	ret = xfer->ret;
> -	if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
> +	cmd = &xfer->cmds[0];
> +	if (!ret)
> +		ccc->dests[0].payload.actual_len = cmd->rx_len;
> +	if (cmd->error == RESPONSE_ERROR_IBA_NACK)
>  		ccc->err = I3C_ERROR_M2;

[Severity: High]
This is a pre-existing issue, but can this lead to a NULL pointer dereference
during a SET CCC command if the target terminates the transfer early?

If a SET CCC (write) command terminates early, the hardware can populate the
response queue with DATA_LEN > 0 (indicating remaining un-sent bytes) and
ERR_STATUS == 0.

Looking at dw_i3c_master_end_xfer_locked():

    if (cmd->rx_len && !cmd->error)
        dw_i3c_master_read_rx_fifo(master, cmd->rx_buf, cmd->rx_len);

Since cmd->rx_buf is uninitialized (NULL) for write transfers, won't this
unconditionally pass a NULL pointer to the FIFO string-read I/O function
when handling the remaining bytes?

>  
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.