usb: typec: tcpm: type-mismatched PDO handling in tcpm_pd_build_request()

usb: typec: tcpm: type-mismatched PDO handling in tcpm_pd_build_request()

[Seungjin Bae]

Hi,

I found that tcpm_pd_select_pdo() can match a source and sink PDO of
different types. Before sending a fix, I would like to ask whether
this type-mixed match is intended, and if not, whether you would
prefer a same-type check in tcpm_pd_select_pdo() or explicit type
handling in tcpm_pd_build_request().

The tcpm_pd_select_pdo() function matches a source PDO against a sink
PDO using their voltage ranges only, without checking that the two
PDOs are of the same type. A source PDO and a sink PDO of different
types (e.g. a Battery source PDO and a Fixed sink PDO) can therefore
be matched as long as their voltage ranges overlap.

tcpm_pd_build_request() then combines the matched pair with
min_power()/min_current(), which apply the same accessor to both
operands. pdo_max_current() and pdo_max_power() decode the same bits
(9:0) of the PDO but scale them differently (x10 mA vs x250 mW), so
when the matched types differ, the sink operand is decoded with the
wrong accessor. This misreads the sink's capability and weakens the
min() bound intended to cap the request to the sink's limit.

This type-mixed match became possible after commit 53fe0de9a35d
("usb: typec: tcpm: pdo matching optimization") relaxed the match to
voltage range only; the min_power()/min_current() macros still assume
a matched pair shares the same type.

I have not observed this on real hardware; I found it by static
analysis.

Thanks,
Seungjin Bae

Re: usb: typec: tcpm: type-mismatched PDO handling in tcpm_pd_build_request()

[Heikki Krogerus]

+Badhri, and also RD and Amit

On Sun, Sep 20, 2026 at 05:29:35PM -0400, Seungjin Bae wrote:
> Hi,
> 
> I found that tcpm_pd_select_pdo() can match a source and sink PDO of
> different types. Before sending a fix, I would like to ask whether
> this type-mixed match is intended, and if not, whether you would
> prefer a same-type check in tcpm_pd_select_pdo() or explicit type
> handling in tcpm_pd_build_request().

I think a patch would have been fine. You can always mark it as RFC if
uncertain.

Thanks,

> The tcpm_pd_select_pdo() function matches a source PDO against a sink
> PDO using their voltage ranges only, without checking that the two
> PDOs are of the same type. A source PDO and a sink PDO of different
> types (e.g. a Battery source PDO and a Fixed sink PDO) can therefore
> be matched as long as their voltage ranges overlap.
> 
> tcpm_pd_build_request() then combines the matched pair with
> min_power()/min_current(), which apply the same accessor to both
> operands. pdo_max_current() and pdo_max_power() decode the same bits
> (9:0) of the PDO but scale them differently (x10 mA vs x250 mW), so
> when the matched types differ, the sink operand is decoded with the
> wrong accessor. This misreads the sink's capability and weakens the
> min() bound intended to cap the request to the sink's limit.
> 
> This type-mixed match became possible after commit 53fe0de9a35d
> ("usb: typec: tcpm: pdo matching optimization") relaxed the match to
> voltage range only; the min_power()/min_current() macros still assume
> a matched pair shares the same type.
> 
> I have not observed this on real hardware; I found it by static
> analysis.

-- 
heikki

Re: usb: typec: tcpm: type-mismatched PDO handling in tcpm_pd_build_request()

[Seungjin Bae]

2026년 9월 22일 (화) 오전 7:15, Heikki Krogerus
<heikki.krogerus@linux.intel.com>님이 작성:
>
> +Badhri, and also RD and Amit
>
> On Sun, Sep 20, 2026 at 05:29:35PM -0400, Seungjin Bae wrote:
> > Hi,
> >
> > I found that tcpm_pd_select_pdo() can match a source and sink PDO of
> > different types. Before sending a fix, I would like to ask whether
> > this type-mixed match is intended, and if not, whether you would
> > prefer a same-type check in tcpm_pd_select_pdo() or explicit type
> > handling in tcpm_pd_build_request().
>
> I think a patch would have been fine. You can always mark it as RFC if
> uncertain.

Thanks, I'll send it as an RFC patch.

Seungjin Bae

>
> Thanks,
>
> > The tcpm_pd_select_pdo() function matches a source PDO against a sink
> > PDO using their voltage ranges only, without checking that the two
> > PDOs are of the same type. A source PDO and a sink PDO of different
> > types (e.g. a Battery source PDO and a Fixed sink PDO) can therefore
> > be matched as long as their voltage ranges overlap.
> >
> > tcpm_pd_build_request() then combines the matched pair with
> > min_power()/min_current(), which apply the same accessor to both
> > operands. pdo_max_current() and pdo_max_power() decode the same bits
> > (9:0) of the PDO but scale them differently (x10 mA vs x250 mW), so
> > when the matched types differ, the sink operand is decoded with the
> > wrong accessor. This misreads the sink's capability and weakens the
> > min() bound intended to cap the request to the sink's limit.
> >
> > This type-mixed match became possible after commit 53fe0de9a35d
> > ("usb: typec: tcpm: pdo matching optimization") relaxed the match to
> > voltage range only; the min_power()/min_current() macros still assume
> > a matched pair shares the same type.
> >
> > I have not observed this on real hardware; I found it by static
> > analysis.
>
> --
> heikki

[PATCH RFC] usb: typec: tcpm: reject type-mismatched source/sink PDO pairs

[pip-izony]

From: Seungjin Bae <eeodqql09@gmail.com>

The tcpm_pd_select_pdo() function matches a source PDO against a sink
PDO using their voltage ranges only, without checking that the two
PDOs are of the same type. A source PDO and a sink PDO of different
types (e.g. a Battery source PDO and a Fixed sink PDO) can therefore
be matched as long as their voltage ranges overlap.

tcpm_pd_build_request() then combines the matched pair with
min_power()/min_current(), which apply the same accessor to both
operands. Since pdo_max_current() and pdo_max_power() decode the same
bits (9:0) with different scaling (x10 mA vs x250 mW), a type-mismatched
sink operand is misinterpreted. This misreads the sink's capability and
weakens the min() bound intended to cap the request to the sink's limit.

Require the source and sink PDO types to match before a pair is
selected. This keeps the voltage-range matching introduced by commit
53fe0de9a35d ("usb: typec: tcpm: pdo matching optimization") and covers
both the min() computation and the mismatch branch in
tcpm_pd_build_request().

I found this by static analysis and have not observed it on hardware, so
I am sending it as RFC.

Fixes: 53fe0de9a35d ("usb: typec: tcpm: pdo matching optimization")
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 2d6b14aa2085..4959872050ae 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -4514,8 +4514,9 @@ static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo,
 				continue;
 			}
 
-			if (max_src_mv <= max_snk_mv &&
-				min_src_mv >= min_snk_mv) {
+			if (pdo_type(port->source_caps[i]) == pdo_type(pdo) &&
+			    max_src_mv <= max_snk_mv &&
+			    min_src_mv >= min_snk_mv) {
 				/* Prefer higher voltages if available */
 				if ((src_mw == max_mw && min_src_mv > max_mv) ||
 							src_mw > max_mw) {
-- 
2.43.0

Re: [PATCH RFC] usb: typec: tcpm: reject type-mismatched source/sink PDO pairs

[Amit Sunil Dhamne]

Hi Seungjin,

On 9/23/26 10:03 AM, pip-izony wrote:
> From: Seungjin Bae <eeodqql09@gmail.com>
> 
> The tcpm_pd_select_pdo() function matches a source PDO against a sink
> PDO using their voltage ranges only, without checking that the two
> PDOs are of the same type. A source PDO and a sink PDO of different
> types (e.g. a Battery source PDO and a Fixed sink PDO) can therefore
> be matched as long as their voltage ranges overlap.
> 
> tcpm_pd_build_request() then combines the matched pair with
> min_power()/min_current(), which apply the same accessor to both
> operands. Since pdo_max_current() and pdo_max_power() decode the same
> bits (9:0) with different scaling (x10 mA vs x250 mW), a type-mismatched
> sink operand is misinterpreted. This misreads the sink's capability and
> weakens the min() bound intended to cap the request to the sink's limit.
> 
> Require the source and sink PDO types to match before a pair is
> selected. 

IMO, you can finish the commit message here. The following bit feels a 
little repetitive.

This keeps the voltage-range matching introduced by commit
> 53fe0de9a35d ("usb: typec: tcpm: pdo matching optimization") and covers
> both the min() computation and the mismatch branch in
> tcpm_pd_build_request().
> 
> I found this by static analysis and have not observed it on hardware, so
> I am sending it as RFC.

Do not include the above ^ sentence in a commit message. It can go in 
the "under the cut" section. Also, you can document any tools used using 
the "Assisted-by:" tag.

> 
> Fixes: 53fe0de9a35d ("usb: typec: tcpm: pdo matching optimization")
> Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>

Please CC stable when you send the actual patch (the non-RFC one).

> ---
>   drivers/usb/typec/tcpm/tcpm.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 2d6b14aa2085..4959872050ae 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -4514,8 +4514,9 @@ static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo,
>   				continue;
>   			}
>   
> -			if (max_src_mv <= max_snk_mv &&
> -				min_src_mv >= min_snk_mv) {
> +			if (pdo_type(port->source_caps[i]) == pdo_type(pdo) &&
nit: we could just use type instead of pdo_type(port->source_caps[i])?

 From a technical standpoint the USB PD 3.2 spec does not explicitly 
prohibit mix type matching. Say, if we match a fixed snk pdo with the 
source's variable type pdo (the rdo structure is the same). However, it 
could have potential issues (instability). For the rest of the PDO types 
the rdo structure is different so obviously you can't match them. It 
shouldn't impact our Pixel user. But I can't say about the others 
especially considering the patch you intended to fix introduced the type 
matching when types were mismatched:

https://lore.kernel.org/all/1521817127-23061-1-git-send-email-jun.li@nxp.com/

Thanks,
Amit

> +			    max_src_mv <= max_snk_mv &&
> +			    min_src_mv >= min_snk_mv) {
>   				/* Prefer higher voltages if available */
>   				if ((src_mw == max_mw && min_src_mv > max_mv) ||
>   							src_mw > max_mw) {