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
prev parent reply other threads:[~2026-09-28 14:15 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-27 21:49 pip-izony
2026-09-28 14:15 ` 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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®