Re: [PATCH v4 1/3] i3c: master: dw: Report actual GET CCC payload length on success
| 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