diff mbox series

[v2] phy: tegra: xusb: Fix use-after-free issue

Message ID 20230508100320.345673-1-haotienh@nvidia.com
State New
Headers
Series [v2] phy: tegra: xusb: Fix use-after-free issue |

Commit Message

HaoTien Hsu May 8, 2023, 10:03 a.m. UTC 
  From: EJ Hsu <ejh@nvidia.com>

For the dual-role port, it will assign the phy dev to usb-phy dev and
use the port dev driver as the dev driver of usb-phy.

When we try to destroy the port dev, it will destroy its dev driver
as well. But we did not remove the reference from usb-phy dev. This
might cause the use-after-free issue in KASAN.

Fixes: e8f7d2f409a1 ("phy: tegra: xusb: Add usb-phy support")
Cc: stable@vger.kernel.org

Signed-off-by: EJ Hsu <ejh@nvidia.com>
Signed-off-by: Haotien Hsu <haotienh@nvidia.com>
---
V1 -> V2: Remove extra movements to clarify the change
---
 drivers/phy/tegra/xusb.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Jon Hunter June 6, 2023, 9:39 a.m. UTC | #1 
Hi Vinod,

On 08/05/2023 11:03, Haotien Hsu wrote:
> From: EJ Hsu <ejh@nvidia.com>
> 
> For the dual-role port, it will assign the phy dev to usb-phy dev and
> use the port dev driver as the dev driver of usb-phy.
> 
> When we try to destroy the port dev, it will destroy its dev driver
> as well. But we did not remove the reference from usb-phy dev. This
> might cause the use-after-free issue in KASAN.
> 
> Fixes: e8f7d2f409a1 ("phy: tegra: xusb: Add usb-phy support")
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: EJ Hsu <ejh@nvidia.com>
> Signed-off-by: Haotien Hsu <haotienh@nvidia.com>
> ---
> V1 -> V2: Remove extra movements to clarify the change
> ---
>   drivers/phy/tegra/xusb.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index 78045bd6c214..26b66a668f3b 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -568,6 +568,7 @@ static void tegra_xusb_port_unregister(struct tegra_xusb_port *port)
>   		usb_role_switch_unregister(port->usb_role_sw);
>   		cancel_work_sync(&port->usb_phy_work);
>   		usb_remove_phy(&port->usb_phy);
> +		port->usb_phy.dev->driver = NULL;
>   	}
>   
>   	if (port->ops->remove)


Are you OK to pick this up now?

FWIW ...

Acked-by: Jon Hunter <jonathanh@nvidia.com>

I believe Thierry already ACK'ed V1.

Jon
Thierry Reding June 6, 2023, 12:35 p.m. UTC | #2 
On Mon, May 08, 2023 at 06:03:20PM +0800, Haotien Hsu wrote:
> From: EJ Hsu <ejh@nvidia.com>
> 
> For the dual-role port, it will assign the phy dev to usb-phy dev and
> use the port dev driver as the dev driver of usb-phy.
> 
> When we try to destroy the port dev, it will destroy its dev driver
> as well. But we did not remove the reference from usb-phy dev. This
> might cause the use-after-free issue in KASAN.
> 
> Fixes: e8f7d2f409a1 ("phy: tegra: xusb: Add usb-phy support")
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: EJ Hsu <ejh@nvidia.com>
> Signed-off-by: Haotien Hsu <haotienh@nvidia.com>
> ---
> V1 -> V2: Remove extra movements to clarify the change
> ---
>  drivers/phy/tegra/xusb.c | 1 +
>  1 file changed, 1 insertion(+)

Haotien,

I had already given an Acked-by on v1. Typically you should add such
tags when you post new versions so that people don't have to repeat
them. Anyway, here it is again:

Acked-by: Thierry Reding <treding@nvidia.com>
HaoTien Hsu June 7, 2023, 2:34 a.m. UTC | #3 
On Tue, 2023-06-06 at 14:35 +0200, Thierry Reding wrote:
> On Mon, May 08, 2023 at 06:03:20PM +0800, Haotien Hsu wrote:
> > From: EJ Hsu <ejh@nvidia.com>
> > 
> > For the dual-role port, it will assign the phy dev to usb-phy dev
> > and
> > use the port dev driver as the dev driver of usb-phy.
> > 
> > When we try to destroy the port dev, it will destroy its dev driver
> > as well. But we did not remove the reference from usb-phy dev. This
> > might cause the use-after-free issue in KASAN.
> > 
> > Fixes: e8f7d2f409a1 ("phy: tegra: xusb: Add usb-phy support")
> > Cc: stable@vger.kernel.org
> > 
> > Signed-off-by: EJ Hsu <ejh@nvidia.com>
> > Signed-off-by: Haotien Hsu <haotienh@nvidia.com>
> > ---
> > V1 -> V2: Remove extra movements to clarify the change
> > ---
> >  drivers/phy/tegra/xusb.c | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Haotien,
> 
> I had already given an Acked-by on v1. Typically you should add such
> tags when you post new versions so that people don't have to repeat
> them. Anyway, here it is again:
> 
> Acked-by: Thierry Reding <treding@nvidia.com>

Hi Thierry,

I see.
My mistake.
Vinod Koul June 8, 2023, 11:48 a.m. UTC | #4 
On 06-06-23, 10:39, Jon Hunter wrote:
> Hi Vinod,
> 
> On 08/05/2023 11:03, Haotien Hsu wrote:
> > From: EJ Hsu <ejh@nvidia.com>
> > 
> > For the dual-role port, it will assign the phy dev to usb-phy dev and
> > use the port dev driver as the dev driver of usb-phy.
> > 
> > When we try to destroy the port dev, it will destroy its dev driver
> > as well. But we did not remove the reference from usb-phy dev. This
> > might cause the use-after-free issue in KASAN.
> > 
> > Fixes: e8f7d2f409a1 ("phy: tegra: xusb: Add usb-phy support")
> > Cc: stable@vger.kernel.org
> > 
> > Signed-off-by: EJ Hsu <ejh@nvidia.com>
> > Signed-off-by: Haotien Hsu <haotienh@nvidia.com>
> > ---
> > V1 -> V2: Remove extra movements to clarify the change
> > ---
> >   drivers/phy/tegra/xusb.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> > index 78045bd6c214..26b66a668f3b 100644
> > --- a/drivers/phy/tegra/xusb.c
> > +++ b/drivers/phy/tegra/xusb.c
> > @@ -568,6 +568,7 @@ static void tegra_xusb_port_unregister(struct tegra_xusb_port *port)
> >   		usb_role_switch_unregister(port->usb_role_sw);
> >   		cancel_work_sync(&port->usb_phy_work);
> >   		usb_remove_phy(&port->usb_phy);
> > +		port->usb_phy.dev->driver = NULL;
> >   	}
> >   	if (port->ops->remove)
> 
> 
> Are you OK to pick this up now?

Changes looks good to me. But title should describe the change, so if
Haotien can change title to reflect the change in patch, I would be
happy to apply

> 
> FWIW ...
> 
> Acked-by: Jon Hunter <jonathanh@nvidia.com>

ofc this should be carried too

> 
> I believe Thierry already ACK'ed V1.
> 
> Jon
> 
> -- 
> nvpublic
diff mbox series

Patch

diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index 78045bd6c214..26b66a668f3b 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -568,6 +568,7 @@  static void tegra_xusb_port_unregister(struct tegra_xusb_port *port)
 		usb_role_switch_unregister(port->usb_role_sw);
 		cancel_work_sync(&port->usb_phy_work);
 		usb_remove_phy(&port->usb_phy);
+		port->usb_phy.dev->driver = NULL;
 	}
 
 	if (port->ops->remove)