| Message ID | 20240514220134.2143181-1-amitsd@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] usb: typec: tcpm: fix use-after-free case in tcpm_register_source_caps | expand |
On Tue, May 14, 2024 at 03:01:31PM GMT, Amit Sunil Dhamne wrote: > There could be a potential use-after-free case in > tcpm_register_source_caps(). This could happen when: > * new (say invalid) source caps are advertised > * the existing source caps are unregistered > * tcpm_register_source_caps() returns with an error as > usb_power_delivery_register_capabilities() fails > > This causes port->partner_source_caps to hold on to the now freed source > caps. > > Reset port->partner_source_caps value to NULL after unregistering > existing source caps. > > Fixes: 230ecdf71a64 ("usb: typec: tcpm: unregister existing source caps before re-registration") > Cc: stable@vger.kernel.org > Signed-off-by: Amit Sunil Dhamne <amitsd@google.com> > Reviewed-by: Ondrej Jirman <megi@xff.cz> Thanks for the fix. Kind regards, o. > > --- > drivers/usb/typec/tcpm/tcpm.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index 8a1af08f71b6..be4127ef84e9 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -3014,8 +3014,10 @@ static int tcpm_register_source_caps(struct tcpm_port *port) > memcpy(caps.pdo, port->source_caps, sizeof(u32) * port->nr_source_caps); > caps.role = TYPEC_SOURCE; > > - if (cap) > + if (cap) { > usb_power_delivery_unregister_capabilities(cap); > + port->partner_source_caps = NULL; > + } > > cap = usb_power_delivery_register_capabilities(port->partner_pd, &caps); > if (IS_ERR(cap)) > > base-commit: 51474ab44abf907023a8a875e799b07de461e466 > -- > 2.45.0.rc1.225.g2a3ae87e7f-goog >
On Tue, May 14, 2024 at 03:01:31PM -0700, Amit Sunil Dhamne wrote: > There could be a potential use-after-free case in > tcpm_register_source_caps(). This could happen when: > * new (say invalid) source caps are advertised > * the existing source caps are unregistered > * tcpm_register_source_caps() returns with an error as > usb_power_delivery_register_capabilities() fails > > This causes port->partner_source_caps to hold on to the now freed source > caps. > > Reset port->partner_source_caps value to NULL after unregistering > existing source caps. > > Fixes: 230ecdf71a64 ("usb: typec: tcpm: unregister existing source caps before re-registration") > Cc: stable@vger.kernel.org > Signed-off-by: Amit Sunil Dhamne <amitsd@google.com> > --- > drivers/usb/typec/tcpm/tcpm.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Tue, May 14, 2024 at 03:01:31PM -0700, Amit Sunil Dhamne wrote: > There could be a potential use-after-free case in > tcpm_register_source_caps(). This could happen when: > * new (say invalid) source caps are advertised > * the existing source caps are unregistered > * tcpm_register_source_caps() returns with an error as > usb_power_delivery_register_capabilities() fails > > This causes port->partner_source_caps to hold on to the now freed source > caps. > > Reset port->partner_source_caps value to NULL after unregistering > existing source caps. > > Fixes: 230ecdf71a64 ("usb: typec: tcpm: unregister existing source caps before re-registration") > Cc: stable@vger.kernel.org > Signed-off-by: Amit Sunil Dhamne <amitsd@google.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/tcpm/tcpm.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index 8a1af08f71b6..be4127ef84e9 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -3014,8 +3014,10 @@ static int tcpm_register_source_caps(struct tcpm_port *port) > memcpy(caps.pdo, port->source_caps, sizeof(u32) * port->nr_source_caps); > caps.role = TYPEC_SOURCE; > > - if (cap) > + if (cap) { > usb_power_delivery_unregister_capabilities(cap); > + port->partner_source_caps = NULL; > + } > > cap = usb_power_delivery_register_capabilities(port->partner_pd, &caps); > if (IS_ERR(cap)) > > base-commit: 51474ab44abf907023a8a875e799b07de461e466 > -- > 2.45.0.rc1.225.g2a3ae87e7f-goog
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index 8a1af08f71b6..be4127ef84e9 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -3014,8 +3014,10 @@ static int tcpm_register_source_caps(struct tcpm_port *port) memcpy(caps.pdo, port->source_caps, sizeof(u32) * port->nr_source_caps); caps.role = TYPEC_SOURCE; - if (cap) + if (cap) { usb_power_delivery_unregister_capabilities(cap); + port->partner_source_caps = NULL; + } cap = usb_power_delivery_register_capabilities(port->partner_pd, &caps); if (IS_ERR(cap))
There could be a potential use-after-free case in tcpm_register_source_caps(). This could happen when: * new (say invalid) source caps are advertised * the existing source caps are unregistered * tcpm_register_source_caps() returns with an error as usb_power_delivery_register_capabilities() fails This causes port->partner_source_caps to hold on to the now freed source caps. Reset port->partner_source_caps value to NULL after unregistering existing source caps. Fixes: 230ecdf71a64 ("usb: typec: tcpm: unregister existing source caps before re-registration") Cc: stable@vger.kernel.org Signed-off-by: Amit Sunil Dhamne <amitsd@google.com> --- drivers/usb/typec/tcpm/tcpm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) base-commit: 51474ab44abf907023a8a875e799b07de461e466