linux-kernel - Re: [PATCH 08/15] usb: ehci: avoid gcc-10 zero-length-bounds warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.44L0.2004302231290.19755-100000@netrider.rowland.org>
Date:   Thu, 30 Apr 2020 22:42:05 -0400 (EDT)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Arnd Bergmann <arnd@...db.de>
cc:     linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        <linux-usb@...r.kernel.org>
Subject: Re: [PATCH 08/15] usb: ehci: avoid gcc-10 zero-length-bounds warning

On Thu, 30 Apr 2020, Arnd Bergmann wrote:

> Building ehci drivers with gcc-10 results in a number of warnings like
> when an zero-length array is accessed:
> 
> drivers/usb/host/ehci-hub.c: In function 'ehci_bus_suspend':
> drivers/usb/host/ehci-hub.c:320:30: error: array subscript 14 is outside the bounds of an interior zero-length array 'u32[0]' {aka 'unsigned int[0]'} [-Werror=zero-length-bounds]
>   320 |    u32 __iomem *hostpc_reg = &ehci->regs->hostpc[port];
>       |                              ^~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/usb/host/ehci.h:273,
>                  from drivers/usb/host/ehci-hcd.c:96:
> include/linux/usb/ehci_def.h:186:7: note: while referencing 'hostpc'
>   186 |  u32  hostpc[0]; /* HOSTPC extension */
>       |       ^~~~~~
> In file included from drivers/usb/host/ehci-hcd.c:305:
> drivers/usb/host/ehci-hub.c: In function 'ehci_hub_control':
> drivers/usb/host/ehci-hub.c:892:15: error: array subscript 256 is outside the bounds of an interior zero-length array 'u32[0]' {aka 'unsigned int[0]'} [-Werror=zero-length-bounds]
>   892 |  hostpc_reg = &ehci->regs->hostpc[temp];
>       |               ^~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/usb/host/ehci.h:273,
>                  from drivers/usb/host/ehci-hcd.c:96:
> include/linux/usb/ehci_def.h:186:7: note: while referencing 'hostpc'
>   186 |  u32  hostpc[0]; /* HOSTPC extension */
>       |       ^~~~~~
> 
> All these fields are colocated with reserved fields that I guess
> refer to the correct field length.

No, they don't.

> Change the two struct definition to use an unnamed union to define
> both of these fields at the same location as the corresponding
> reserved fields.
> 
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  include/linux/usb/ehci_def.h | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/usb/ehci_def.h b/include/linux/usb/ehci_def.h
> index 78e006355557..8777d8e56ef2 100644
> --- a/include/linux/usb/ehci_def.h
> +++ b/include/linux/usb/ehci_def.h
> @@ -127,7 +127,8 @@ struct ehci_regs {
>  #define FLAG_CF		(1<<0)		/* true: we'll support "high speed" */
>  
>  	/* PORTSC: offset 0x44 */
> -	u32		port_status[0];	/* up to N_PORTS */
> +	union {
> +		u32		port_status[9];	/* up to N_PORTS */

This array can have up to 15 elements, meaning that it can extend out
to offset 0x80.

>  /* EHCI 1.1 addendum */
>  #define PORTSC_SUSPEND_STS_ACK 0
>  #define PORTSC_SUSPEND_STS_NYET 1
> @@ -165,7 +166,8 @@ struct ehci_regs {
>  #define PORT_CONNECT	(1<<0)		/* device connected */
>  #define PORT_RWC_BITS   (PORT_CSC | PORT_PEC | PORT_OCC)
>  
> -	u32		reserved3[9];
> +		u32		reserved3[9];
> +	};
>  
>  	/* USBMODE: offset 0x68 */
>  	u32		usbmode;	/* USB Device mode */

As you see, this next field actually lies inside the preceding array.  
It's not a real conflict; any hardware which supports the usbmode field 
uses only the first element of the port_status array.

I don't know how you want to handle this.  Doing

#define usbmode port_status[9]

doesn't seem like a very good approach, but I can't think of anything 
better at the moment.  Maybe just set the array size to 9, as you did, 
but with a comment explaining what's really going on.

> @@ -181,11 +183,13 @@ struct ehci_regs {
>   * PORTSCx
>   */
>  	/* HOSTPC: offset 0x84 */
> -	u32		hostpc[0];	/* HOSTPC extension */
> +	union {
> +		u32		hostpc[17];	/* HOSTPC extension */

Likewise, this array can have up to 15 elements.  In fact, it's the 
same size as the port_status array.

>  #define HOSTPC_PHCD	(1<<22)		/* Phy clock disable */
>  #define HOSTPC_PSPD	(3<<25)		/* Port speed detection */
>  
> -	u32		reserved5[17];
> +		u32		reserved5[17];
> +	};
>  
>  	/* USBMODE_EX: offset 0xc8 */
>  	u32		usbmode_ex;	/* USB Device mode extension */

Alan Stern