Linux USB
 help / color / mirror / Atom feed
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable <stable@kernel.org>
Subject: Re: [PATCH 1/8] usb: typec: wcove: don't write past struct pd_message in wcove_read_rx_buffer()
Date: Mon, 18 May 2026 13:55:53 +0300	[thread overview]
Message-ID: <agrwOYXfdXm6mn00@kuha> (raw)
In-Reply-To: <2026051347-clustered-deflected-9543@gregkh>

On Wed, May 13, 2026 at 05:52:48PM +0200, Greg Kroah-Hartman wrote:
> wcove_read_rx_buffer() copies the PD RX FIFO into the caller's
> struct pd_message with
> 
> 	for (i = 0; i < USBC_RXINFO_RXBYTES(info); i++)
> 		regmap_read(wcove->regmap, USBC_RX_DATA + i, msg + i);
> 
> which has two problems:
> 
> USBC_RXINFO_RXBYTES() is a 5-bit field (max 31) while struct pd_message
> is 30 bytes (__le16 header + __le32 payload[PD_MAX_PAYLOAD], packed).
> The byte count latched in RXINFO is the number of bytes the port partner
> put on the wire, so a malicious partner that transmits a 31-byte frame
> can drive the loop one byte past the destination if the WCOVE BMC
> receiver does not enforce the PD object-count limit in hardware. The
> existing FIXME flagged this as unverified.
> 
> Independently, regmap_read() takes an unsigned int * and stores a full
> unsigned int at the destination. Passing the byte pointer msg + i means
> each iteration writes four bytes; the high three are zero (val_bits is
> 8) and are normally overwritten by the next iteration, but the final
> iteration's high bytes are not. With RXBYTES == 30 the i == 29 iteration
> already writes three zero bytes past msg, which sits on the IRQ thread's
> stack in wcove_typec_irq().
> 
> Clamp the loop to sizeof(struct pd_message) and read each register into
> a local before storing only its low byte, so the copy can never exceed
> the destination regardless of what RXINFO reports.
> 
> Assisted-by: gkh_clanker_t1000
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/tcpm/wcove.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/wcove.c b/drivers/usb/typec/tcpm/wcove.c
> index 759c982bb16a..0e5a3e277c3e 100644
> --- a/drivers/usb/typec/tcpm/wcove.c
> +++ b/drivers/usb/typec/tcpm/wcove.c
> @@ -444,9 +444,11 @@ static int wcove_start_toggling(struct tcpc_dev *tcpc,
>  	return regmap_write(wcove->regmap, USBC_CONTROL1, usbc_ctrl);
>  }
>  
> -static int wcove_read_rx_buffer(struct wcove_typec *wcove, void *msg)
> +static int wcove_read_rx_buffer(struct wcove_typec *wcove,
> +				struct pd_message *msg)
>  {
> -	unsigned int info;
> +	unsigned int info, val, len;
> +	u8 *buf = (u8 *)msg;
>  	int ret;
>  	int i;
>  
> @@ -454,12 +456,13 @@ static int wcove_read_rx_buffer(struct wcove_typec *wcove, void *msg)
>  	if (ret)
>  		return ret;
>  
> -	/* FIXME: Check that USBC_RXINFO_RXBYTES(info) matches the header */
> +	len = min(USBC_RXINFO_RXBYTES(info), sizeof(*msg));
>  
> -	for (i = 0; i < USBC_RXINFO_RXBYTES(info); i++) {
> -		ret = regmap_read(wcove->regmap, USBC_RX_DATA + i, msg + i);
> +	for (i = 0; i < len; i++) {
> +		ret = regmap_read(wcove->regmap, USBC_RX_DATA + i, &val);
>  		if (ret)
>  			return ret;
> +		buf[i] = val;
>  	}
>  
>  	return regmap_write(wcove->regmap, USBC_RXSTATUS,
> -- 
> 2.54.0

-- 
heikki

      parent reply	other threads:[~2026-05-18 10:55 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13 15:52 [PATCH 1/8] usb: typec: wcove: don't write past struct pd_message in wcove_read_rx_buffer() Greg Kroah-Hartman
2026-05-13 15:52 ` [PATCH 2/8] usb: typec: altmodes/displayport: validate count before reading Status Update VDO Greg Kroah-Hartman
2026-05-18 11:07   ` Heikki Krogerus
2026-05-19 20:02   ` Abhishek Pandit-Subedi
2026-05-13 15:52 ` [PATCH 3/8] usb: typec: tcpm/tcpci_maxim: validate header NDO against RX_BYTE_CNT Greg Kroah-Hartman
2026-05-13 15:52 ` [PATCH 4/8] usb: typec: tcpm: validate VDO count in Discover Identity ACK handlers Greg Kroah-Hartman
2026-05-15  5:13   ` Badhri Jagan Sridharan
2026-05-13 15:52 ` [PATCH 5/8] usb: typec: tcpm: validate VDO count before reading Attention status VDO Greg Kroah-Hartman
2026-05-14 21:55   ` Badhri Jagan Sridharan
2026-05-15  5:05     ` Greg Kroah-Hartman
2026-05-13 15:52 ` [PATCH 6/8] usb: typec: tcpm: bound altmode_desc[] per iteration in svdm_consume_modes() Greg Kroah-Hartman
2026-05-13 15:52 ` [PATCH 7/8] usb: typec: ucsi: displayport: NAK DP_CMD_CONFIGURE without a payload VDO Greg Kroah-Hartman
2026-05-18 11:13   ` Heikki Krogerus
2026-05-19 20:02   ` Abhishek Pandit-Subedi
2026-05-13 15:52 ` [PATCH 8/8] usb: typec: ucsi: validate connector number in ucsi_connector_change() Greg Kroah-Hartman
2026-05-13 16:03   ` Benson Leung
2026-05-13 16:13   ` Abel Vesa
2026-05-18 11:16   ` Heikki Krogerus
2026-05-19 20:00   ` Abhishek Pandit-Subedi
2026-05-18 10:55 ` Heikki Krogerus [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agrwOYXfdXm6mn00@kuha \
    --to=heikki.krogerus@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox