| Message ID | 8139e4cc-4e5c-40e2-9c4b-717ad3215868@rowland.harvard.edu |
|---|---|
| State | New |
| Headers | show |
| Series | usb: ehci-fsl: Fix use of private data to avoid -Wflex-array-member-not-at-end warning | expand |
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" <gustavoars@kernel.org> > Signed-off-by: Alan Stern <stern@rowland.harvard.edu> > Link: https://lore.kernel.org/linux-usb/Z-R9BcnSzrRv5FX_@kspp/ Reviewed-by: Kees Cook <kees@kernel.org> -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, > }; >
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, };
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. Reported-by: "Gustavo A. R. Silva" <gustavoars@kernel.org> Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Link: https://lore.kernel.org/linux-usb/Z-R9BcnSzrRv5FX_@kspp/ --- drivers/usb/host/ehci-fsl.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-)