On Thu, Mar 27, 2025 at 03:31:15PM -0400, Alan Stern wrote: > In the course of fixing up the usages of flexible arrays, Gustavo > submitted a patch updating the ehci-fsl driver. However, the patch > was wrong because the driver was using the .priv member of the > ehci_hcd structure incorrectly. The private data is not supposed to > be a wrapper containing the ehci_hcd structure; it is supposed to be a > sub-structure stored in the .priv member. > > Fix the problem by replacing the ehci_fsl structure with > ehci_fsl_priv, containing only the private data, along with a suitable > conversion macro for accessing it. This removes the problem of having > data follow a flexible array member.
Thanks for figuring out the right solution! :) > > Reported-by: "Gustavo A. R. Silva" <[email protected]> > Signed-off-by: Alan Stern <[email protected]> > Link: https://lore.kernel.org/linux-usb/Z-R9BcnSzrRv5FX_@kspp/ Reviewed-by: Kees Cook <[email protected]> -Kees > > --- > > drivers/usb/host/ehci-fsl.c | 25 ++++++++----------------- > 1 file changed, 8 insertions(+), 17 deletions(-) > > Index: usb-devel/drivers/usb/host/ehci-fsl.c > =================================================================== > --- usb-devel.orig/drivers/usb/host/ehci-fsl.c > +++ usb-devel/drivers/usb/host/ehci-fsl.c > @@ -410,15 +410,13 @@ static int ehci_fsl_setup(struct usb_hcd > return retval; > } > > -struct ehci_fsl { > - struct ehci_hcd ehci; > - > -#ifdef CONFIG_PM > +struct ehci_fsl_priv { > /* Saved USB PHY settings, need to restore after deep sleep. */ > u32 usb_ctrl; > -#endif > }; > > +#define hcd_to_ehci_fsl_priv(h) ((struct ehci_fsl_priv *) > hcd_to_ehci(h)->priv) > + > #ifdef CONFIG_PM > > #ifdef CONFIG_PPC_MPC512x > @@ -566,17 +564,10 @@ static inline int ehci_fsl_mpc512x_drv_r > } > #endif /* CONFIG_PPC_MPC512x */ > > -static struct ehci_fsl *hcd_to_ehci_fsl(struct usb_hcd *hcd) > -{ > - struct ehci_hcd *ehci = hcd_to_ehci(hcd); > - > - return container_of(ehci, struct ehci_fsl, ehci); > -} > - > static int ehci_fsl_drv_suspend(struct device *dev) > { > struct usb_hcd *hcd = dev_get_drvdata(dev); > - struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd); > + struct ehci_fsl_priv *priv = hcd_to_ehci_fsl_priv(hcd); > void __iomem *non_ehci = hcd->regs; > > if (of_device_is_compatible(dev->parent->of_node, > @@ -589,14 +580,14 @@ static int ehci_fsl_drv_suspend(struct d > if (!fsl_deep_sleep()) > return 0; > > - ehci_fsl->usb_ctrl = ioread32be(non_ehci + FSL_SOC_USB_CTRL); > + priv->usb_ctrl = ioread32be(non_ehci + FSL_SOC_USB_CTRL); > return 0; > } > > static int ehci_fsl_drv_resume(struct device *dev) > { > struct usb_hcd *hcd = dev_get_drvdata(dev); > - struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd); > + struct ehci_fsl_priv *priv = hcd_to_ehci_fsl_priv(hcd); > struct ehci_hcd *ehci = hcd_to_ehci(hcd); > void __iomem *non_ehci = hcd->regs; > > @@ -612,7 +603,7 @@ static int ehci_fsl_drv_resume(struct de > usb_root_hub_lost_power(hcd->self.root_hub); > > /* Restore USB PHY settings and enable the controller. */ > - iowrite32be(ehci_fsl->usb_ctrl, non_ehci + FSL_SOC_USB_CTRL); > + iowrite32be(priv->usb_ctrl, non_ehci + FSL_SOC_USB_CTRL); > > ehci_reset(ehci); > ehci_fsl_reinit(ehci); > @@ -671,7 +662,7 @@ static int ehci_start_port_reset(struct > #endif /* CONFIG_USB_OTG */ > > static const struct ehci_driver_overrides ehci_fsl_overrides __initconst = { > - .extra_priv_size = sizeof(struct ehci_fsl), > + .extra_priv_size = sizeof(struct ehci_fsl_priv), > .reset = ehci_fsl_setup, > }; > -- Kees Cook
