From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: pip-izony <eeodqql09@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Saranya Gopal <saranya.gopal@intel.com>,
Rajaram Regupathy <rajaram.regupathy@intel.com>,
Benson Leung <bleung@chromium.org>,
Andrei Kuchynski <akuchynski@chromium.org>,
Jameson Thies <jthies@google.com>,
Kyungtae Kim <Kyungtae.Kim@dartmouth.edu>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH] usb: typec: ucsi: limit the PDO count to the number of PDOs requested
Date: Mon, 28 Sep 2026 16:15:14 +0200 [thread overview]
Message-ID: <arp2cstT7184pH66@black.igk.intel.com> (raw)
In-Reply-To: <20260927214904.447250-2-eeodqql09@gmail.com>
On Sun, Sep 27, 2026 at 05:49:05PM -0400, pip-izony wrote:
> From: Seungjin Bae <eeodqql09@gmail.com>
>
> In ucsi_get_pdos(), the number of PDOs is derived from the data length
> reported in the CCI register, which is provided by the PPM firmware.
>
> The function requests at most UCSI_MAX_PDOS PDOs on the first read and
> PDO_MAX_OBJECTS - UCSI_MAX_PDOS on the second, and ucsi_send_command()
> only copies that many bytes into the buffer. However, the returned
> length is taken from the 8 bit CCI data length field and is not bounded
> by the size of the request.
>
> If a malicious PPM reports a larger length, e.g. 0xFF, each
> read is counted as 63 PDOs and ucsi_get_pdos() returns up to 126, beyond
> PDO_MAX_OBJECTS and the number of PDOs actually read.
>
> ucsi_get_src_pdos() stores this value in con->num_pdos, and
> ucsi_psy_get_voltage_max() and ucsi_psy_get_current_max() use it to
> index con->src_pdos[con->num_pdos - 1], resulting in an out-of-bounds
> read. This happens without any userspace action, since
> ucsi_get_src_pdos() calls ucsi_port_psy_changed() and the resulting
> uevent reads every property.
>
> Fix this by limiting the count of each read to the number of PDOs
> requested, so that the returned value always matches the buffer
> contents and never exceeds PDO_MAX_OBJECTS.
I'm not going to accept any more changes like this that silently "fix"
real or hypothetical firmware issues.
In stead of doing that, make the code print a big fat error message
informing the user that the FW/PPM sucks in the system, and then
return a failure from the function.
Thanks,
> Fixes: b04e1747fbcc ("usb: typec: ucsi: Register USB Power Delivery Capabilities")
> Cc: stable@vger.kernel.org
> Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
> ---
> drivers/usb/typec/ucsi/ucsi.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index bef3f9b71d71..639f99f49ff9 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -898,7 +898,8 @@ static int ucsi_get_pdos(struct ucsi_connector *con, enum typec_role role,
> if (ret < 0)
> return ret;
>
> - num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> + /* The PPM may report more data than was requested */
> + num_pdos = min_t(u8, ret / sizeof(u32), UCSI_MAX_PDOS);
> if (num_pdos < UCSI_MAX_PDOS)
> return num_pdos;
>
> @@ -908,7 +909,8 @@ static int ucsi_get_pdos(struct ucsi_connector *con, enum typec_role role,
> if (ret < 0)
> return ret;
>
> - return ret / sizeof(u32) + num_pdos;
> + return min_t(u8, ret / sizeof(u32),
> + PDO_MAX_OBJECTS - UCSI_MAX_PDOS) + num_pdos;
> }
>
> static int ucsi_get_src_pdos(struct ucsi_connector *con)
> --
> 2.43.0
--
heikki
next prev parent reply other threads:[~2026-09-28 14:15 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-27 21:49 [PATCH] usb: typec: ucsi: limit the PDO count to the number of PDOs requested pip-izony
2026-09-28 14:15 ` Heikki Krogerus [this message]
2026-09-29 13:32 ` Greg Kroah-Hartman
2026-10-02 20:21 ` [PATCH v2] usb: typec: ucsi: reject GET_PDOS responses longer than requested pip-izony
2026-10-09 11:21 ` Heikki Krogerus
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=arp2cstT7184pH66@black.igk.intel.com \
--to=heikki.krogerus@linux.intel.com \
--cc=Kyungtae.Kim@dartmouth.edu \
--cc=akuchynski@chromium.org \
--cc=bleung@chromium.org \
--cc=eeodqql09@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=jthies@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=rajaram.regupathy@intel.com \
--cc=saranya.gopal@intel.com \
--cc=stable@vger.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.