The subject is too long. You've sent v2, v3, and v4 today. Please, wait
for a day between resends.
On Wed, Dec 18, 2024 at 09:59:32AM +0100, Miao.Zhu wrote:
> static int tcpci_set_pd_rx(struct tcpc_dev *tcpc, bool enable)
> @@ -741,33 +748,86 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
> struct pd_message msg;
> unsigned int cnt, payload_cnt;
> u16 header;
> + unsigned int frame_type;
> + enum tcpm_transmit_type rx_type;
>
> regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
> /*
> * 'cnt' corresponds to READABLE_BYTE_COUNT in section 4.4.14
> * of the TCPCI spec [Rev 2.0 Ver 1.0 October 2017] and is
> * defined in table 4-36 as one greater than the number of
> - * bytes received. And that number includes the header. So:
> + * bytes received. And that number includes the header.
> + * In Section 4.4.14 of the TCPCI spec [Rev 2.0 Ver 1.0 October, 2017],
> + * the RECEIVE_BUFFER comprises of three sets of registers:
> + * READABLE_BYTE_COUNT, RX_BUF_FRAME_TYPE and RX_BUF_BYTE_x.
> + * These registers can only be accessed by reading at a common
> + * register address 0x30h.
> */
> - if (cnt > 3)
> - payload_cnt = cnt - (1 + sizeof(msg.header));
> - else
> - payload_cnt = 0;
> + if (tcpci->data->RX_BUF_BYTE_x_hidden) {
> + u8 buf[TCPC_RECEIVE_BUFFER_MAX_LEN] = {0,};
> + u8 pos = 0;
> +
> + /* Read the count and frame type in RECEIVE_BUFFER */
> + regmap_raw_read(tcpci->regmap, TCPC_RX_BYTE_CNT, buf, 2);
> + /* READABLE_BYTE_COUNT */
> + cnt = buf[0];
> + /* RX_BUF_FRAME_TYPE */
> + frame_type = buf[1];
> +
> + /* Read the content of the USB PD message in RECEIVE_BUFFER */
> + regmap_raw_read(tcpci->regmap, TCPC_RX_BYTE_CNT, buf, cnt + 1);
^^^
buffer overflow?
> +
> + pos += 2;
> + memcpy(&msg.header, &buf[pos], sizeof(msg.header));
> +
> + if (cnt > 3) {
> + pos += sizeof(msg.header);
> + payload_cnt = cnt - (1 + sizeof(msg.header));
> + if (WARN_ON(payload_cnt > sizeof(msg.payload)))
> + payload_cnt = sizeof(msg.payload);
> + memcpy(&msg.payload, &buf[pos], payload_cnt);
There is existing code later which does bounds checking on payload_cnt,
but it's too late. We would have already overflowed buf[] and
msg.payload here.
> + }
> + } else {
> + regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
> + /*
> + * 'cnt' corresponds to READABLE_BYTE_COUNT in section 4.4.14
> + * of the TCPCI spec [Rev 2.0 Ver 1.0 October 2017] and is
> + * defined in table 4-36 as one greater than the number of
> + * bytes received. And that number includes the header. So:
> + */
> + if (cnt > 3)
> + payload_cnt = cnt - (1 + sizeof(msg.header));
> + else
> + payload_cnt = 0;
>
> - tcpci_read16(tcpci, TCPC_RX_HDR, &header);
> - msg.header = cpu_to_le16(header);
> + regmap_read(tcpci->regmap, TCPC_RX_BUF_FRAME_TYPE, &frame_type);
>
> - if (WARN_ON(payload_cnt > sizeof(msg.payload)))
> - payload_cnt = sizeof(msg.payload);
regards,
dan carpenter
