[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <202504071215.DED5FA3535@keescook>
Date: Mon, 7 Apr 2025 12:15:33 -0700
From: Kees Cook <kees@...nel.org>
To: Alan Stern <stern@...land.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Gustavo A. R. Silva" <gustavoars@...nel.org>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org
Subject: Re: [PATCH] usb: ehci-fsl: Fix use of private data to avoid
-Wflex-array-member-not-at-end warning
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@...nel.org>
> Signed-off-by: Alan Stern <stern@...land.harvard.edu>
> Link: https://lore.kernel.org/linux-usb/Z-R9BcnSzrRv5FX_@kspp/
Reviewed-by: Kees Cook <kees@...nel.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,
> };
>
--
Kees Cook
Powered by blists - more mailing lists