* [PATCH v2] usb: typec: tcpm: Correct the PDO counting in pd_set
@ 2024-03-26 15:19 Kyle Tso
2024-03-27 11:10 ` Heikki Krogerus
2024-04-04 13:22 ` Greg KH
0 siblings, 2 replies; 4+ messages in thread
From: Kyle Tso @ 2024-03-26 15:19 UTC (permalink / raw
To: linux, heikki.krogerus, gregkh
Cc: badhri, linux-kernel, linux-usb, Kyle Tso, stable
Off-by-one errors happen because nr_snk_pdo and nr_src_pdo are
incorrectly added one. The index of the loop is equal to the number of
PDOs to be updated when leaving the loop and it doesn't need to be added
one.
When doing the power negotiation, TCPM relies on the "nr_snk_pdo" as
the size of the local sink PDO array to match the Source capabilities
of the partner port. If the off-by-one overflow occurs, a wrong RDO
might be sent and unexpected power transfer might happen such as over
voltage or over current (than expected).
"nr_src_pdo" is used to set the Rp level when the port is in Source
role. It is also the array size of the local Source capabilities when
filling up the buffer which will be sent as the Source PDOs (such as
in Power Negotiation). If the off-by-one overflow occurs, a wrong Rp
level might be set and wrong Source PDOs will be sent to the partner
port. This could potentially cause over current or port resets.
Fixes: cd099cde4ed2 ("usb: typec: tcpm: Support multiple capabilities")
Cc: stable@vger.kernel.org
Signed-off-by: Kyle Tso <kyletso@google.com>
---
v1 -> v2:
- update the commit message (adding the problems this patch solves)
drivers/usb/typec/tcpm/tcpm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index ae2b6c94482d..2464710ea0c8 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -6855,14 +6855,14 @@ static int tcpm_pd_set(struct typec_port *p, struct usb_power_delivery *pd)
if (data->sink_desc.pdo[0]) {
for (i = 0; i < PDO_MAX_OBJECTS && data->sink_desc.pdo[i]; i++)
port->snk_pdo[i] = data->sink_desc.pdo[i];
- port->nr_snk_pdo = i + 1;
+ port->nr_snk_pdo = i;
port->operating_snk_mw = data->operating_snk_mw;
}
if (data->source_desc.pdo[0]) {
for (i = 0; i < PDO_MAX_OBJECTS && data->source_desc.pdo[i]; i++)
port->snk_pdo[i] = data->source_desc.pdo[i];
- port->nr_src_pdo = i + 1;
+ port->nr_src_pdo = i;
}
switch (port->state) {
--
2.44.0.396.g6e790dbe36-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] usb: typec: tcpm: Correct the PDO counting in pd_set
2024-03-26 15:19 [PATCH v2] usb: typec: tcpm: Correct the PDO counting in pd_set Kyle Tso
@ 2024-03-27 11:10 ` Heikki Krogerus
2024-04-04 13:22 ` Greg KH
1 sibling, 0 replies; 4+ messages in thread
From: Heikki Krogerus @ 2024-03-27 11:10 UTC (permalink / raw
To: Kyle Tso; +Cc: linux, gregkh, badhri, linux-kernel, linux-usb, stable
On Tue, Mar 26, 2024 at 11:19:09PM +0800, Kyle Tso wrote:
> Off-by-one errors happen because nr_snk_pdo and nr_src_pdo are
> incorrectly added one. The index of the loop is equal to the number of
> PDOs to be updated when leaving the loop and it doesn't need to be added
> one.
>
> When doing the power negotiation, TCPM relies on the "nr_snk_pdo" as
> the size of the local sink PDO array to match the Source capabilities
> of the partner port. If the off-by-one overflow occurs, a wrong RDO
> might be sent and unexpected power transfer might happen such as over
> voltage or over current (than expected).
>
> "nr_src_pdo" is used to set the Rp level when the port is in Source
> role. It is also the array size of the local Source capabilities when
> filling up the buffer which will be sent as the Source PDOs (such as
> in Power Negotiation). If the off-by-one overflow occurs, a wrong Rp
> level might be set and wrong Source PDOs will be sent to the partner
> port. This could potentially cause over current or port resets.
>
> Fixes: cd099cde4ed2 ("usb: typec: tcpm: Support multiple capabilities")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kyle Tso <kyletso@google.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> v1 -> v2:
> - update the commit message (adding the problems this patch solves)
>
> drivers/usb/typec/tcpm/tcpm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index ae2b6c94482d..2464710ea0c8 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -6855,14 +6855,14 @@ static int tcpm_pd_set(struct typec_port *p, struct usb_power_delivery *pd)
> if (data->sink_desc.pdo[0]) {
> for (i = 0; i < PDO_MAX_OBJECTS && data->sink_desc.pdo[i]; i++)
> port->snk_pdo[i] = data->sink_desc.pdo[i];
> - port->nr_snk_pdo = i + 1;
> + port->nr_snk_pdo = i;
> port->operating_snk_mw = data->operating_snk_mw;
> }
>
> if (data->source_desc.pdo[0]) {
> for (i = 0; i < PDO_MAX_OBJECTS && data->source_desc.pdo[i]; i++)
> port->snk_pdo[i] = data->source_desc.pdo[i];
> - port->nr_src_pdo = i + 1;
> + port->nr_src_pdo = i;
> }
>
> switch (port->state) {
> --
> 2.44.0.396.g6e790dbe36-goog
--
heikki
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] usb: typec: tcpm: Correct the PDO counting in pd_set
2024-03-26 15:19 [PATCH v2] usb: typec: tcpm: Correct the PDO counting in pd_set Kyle Tso
2024-03-27 11:10 ` Heikki Krogerus
@ 2024-04-04 13:22 ` Greg KH
2024-04-04 13:39 ` Kyle Tso
1 sibling, 1 reply; 4+ messages in thread
From: Greg KH @ 2024-04-04 13:22 UTC (permalink / raw
To: Kyle Tso; +Cc: linux, heikki.krogerus, badhri, linux-kernel, linux-usb, stable
On Tue, Mar 26, 2024 at 11:19:09PM +0800, Kyle Tso wrote:
> Off-by-one errors happen because nr_snk_pdo and nr_src_pdo are
> incorrectly added one. The index of the loop is equal to the number of
> PDOs to be updated when leaving the loop and it doesn't need to be added
> one.
>
> When doing the power negotiation, TCPM relies on the "nr_snk_pdo" as
> the size of the local sink PDO array to match the Source capabilities
> of the partner port. If the off-by-one overflow occurs, a wrong RDO
> might be sent and unexpected power transfer might happen such as over
> voltage or over current (than expected).
>
> "nr_src_pdo" is used to set the Rp level when the port is in Source
> role. It is also the array size of the local Source capabilities when
> filling up the buffer which will be sent as the Source PDOs (such as
> in Power Negotiation). If the off-by-one overflow occurs, a wrong Rp
> level might be set and wrong Source PDOs will be sent to the partner
> port. This could potentially cause over current or port resets.
>
> Fixes: cd099cde4ed2 ("usb: typec: tcpm: Support multiple capabilities")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kyle Tso <kyletso@google.com>
> ---
> v1 -> v2:
> - update the commit message (adding the problems this patch solves)
>
> drivers/usb/typec/tcpm/tcpm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
This fails to apply to my usb-linus branch :(
Can you rebase and resend?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] usb: typec: tcpm: Correct the PDO counting in pd_set
2024-04-04 13:22 ` Greg KH
@ 2024-04-04 13:39 ` Kyle Tso
0 siblings, 0 replies; 4+ messages in thread
From: Kyle Tso @ 2024-04-04 13:39 UTC (permalink / raw
To: Greg KH; +Cc: linux, heikki.krogerus, badhri, linux-kernel, linux-usb, stable
On Thu, Apr 4, 2024 at 9:22 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Mar 26, 2024 at 11:19:09PM +0800, Kyle Tso wrote:
> > Off-by-one errors happen because nr_snk_pdo and nr_src_pdo are
> > incorrectly added one. The index of the loop is equal to the number of
> > PDOs to be updated when leaving the loop and it doesn't need to be added
> > one.
> >
> > When doing the power negotiation, TCPM relies on the "nr_snk_pdo" as
> > the size of the local sink PDO array to match the Source capabilities
> > of the partner port. If the off-by-one overflow occurs, a wrong RDO
> > might be sent and unexpected power transfer might happen such as over
> > voltage or over current (than expected).
> >
> > "nr_src_pdo" is used to set the Rp level when the port is in Source
> > role. It is also the array size of the local Source capabilities when
> > filling up the buffer which will be sent as the Source PDOs (such as
> > in Power Negotiation). If the off-by-one overflow occurs, a wrong Rp
> > level might be set and wrong Source PDOs will be sent to the partner
> > port. This could potentially cause over current or port resets.
> >
> > Fixes: cd099cde4ed2 ("usb: typec: tcpm: Support multiple capabilities")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Kyle Tso <kyletso@google.com>
> > ---
> > v1 -> v2:
> > - update the commit message (adding the problems this patch solves)
> >
> > drivers/usb/typec/tcpm/tcpm.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> This fails to apply to my usb-linus branch :(
>
> Can you rebase and resend?
>
> thanks,
>
> greg k-h
Just sent v3
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-04 13:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-26 15:19 [PATCH v2] usb: typec: tcpm: Correct the PDO counting in pd_set Kyle Tso
2024-03-27 11:10 ` Heikki Krogerus
2024-04-04 13:22 ` Greg KH
2024-04-04 13:39 ` Kyle Tso
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.