summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKees Cook <keescook@chromium.org>2021-08-18 10:30:17 -0700
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2021-08-18 22:28:27 +0200
commit72dd1843232c9de48e21dc1c85d169fe5328e52e (patch)
treec8efac48e0fee1c342f6cce8611cade21a67a031
parent2af0c5ffadaf9d13eca28409d4238b4e672942d3 (diff)
USB: EHCI: Add register array bounds to HCS ports
The original EHCI register struct used a trailing 0-element array for addressing the N_PORTS-many available registers. However, after commit a46af4ebf9ff ("USB: EHCI: define extension registers like normal ones") the 0-element array started to overlap the USBMODE extension register. To avoid future compile-time warnings about accessing indexes within a 0-element array, rearrange the struct to actually describe the expected layout (max 15 registers) with a union. All offsets remain the same, and bounds checking becomes possible on accesses to port_status and hostpc. There are no binary differences, and struct offsets continue to match. "pahole --hex -C ehci_regs" before: struct ehci_regs { u32 command; /* 0 0x4 */ u32 status; /* 0x4 0x4 */ u32 intr_enable; /* 0x8 0x4 */ u32 frame_index; /* 0xc 0x4 */ u32 segment; /* 0x10 0x4 */ u32 frame_list; /* 0x14 0x4 */ u32 async_next; /* 0x18 0x4 */ u32 reserved1[2]; /* 0x1c 0x8 */ u32 txfill_tuning; /* 0x24 0x4 */ u32 reserved2[6]; /* 0x28 0x18 */ /* --- cacheline 1 boundary (64 bytes) --- */ u32 configured_flag; /* 0x40 0x4 */ u32 port_status[0]; /* 0x44 0 */ u32 reserved3[9]; /* 0x44 0x24 */ u32 usbmode; /* 0x68 0x4 */ u32 reserved4[6]; /* 0x6c 0x18 */ /* --- cacheline 2 boundary (128 bytes) was 4 bytes ago --- */ u32 hostpc[0]; /* 0x84 0 */ u32 reserved5[17]; /* 0x84 0x44 */ /* --- cacheline 3 boundary (192 bytes) was 8 bytes ago --- */ u32 usbmode_ex; /* 0xc8 0x4 */ /* size: 204, cachelines: 4, members: 18 */ /* last cacheline: 12 bytes */ }; after: struct ehci_regs { u32 command; /* 0 0x4 */ u32 status; /* 0x4 0x4 */ u32 intr_enable; /* 0x8 0x4 */ u32 frame_index; /* 0xc 0x4 */ u32 segment; /* 0x10 0x4 */ u32 frame_list; /* 0x14 0x4 */ u32 async_next; /* 0x18 0x4 */ u32 reserved1[2]; /* 0x1c 0x8 */ u32 txfill_tuning; /* 0x24 0x4 */ u32 reserved2[6]; /* 0x28 0x18 */ /* --- cacheline 1 boundary (64 bytes) --- */ u32 configured_flag; /* 0x40 0x4 */ union { u32 port_status[15]; /* 0x44 0x3c */ struct { u32 reserved3[9]; /* 0x44 0x24 */ u32 usbmode; /* 0x68 0x4 */ }; /* 0x44 0x28 */ }; /* 0x44 0x3c */ /* --- cacheline 2 boundary (128 bytes) --- */ u32 reserved4; /* 0x80 0x4 */ u32 hostpc[15]; /* 0x84 0x3c */ /* --- cacheline 3 boundary (192 bytes) --- */ u32 reserved5[2]; /* 0xc0 0x8 */ u32 usbmode_ex; /* 0xc8 0x4 */ /* size: 204, cachelines: 4, members: 16 */ /* last cacheline: 12 bytes */ }; With this fixed, adding -Wzero-length-bounds to the build no longer produces several warnings like this: In file included from drivers/usb/host/ehci-hcd.c:306: drivers/usb/host/ehci-hub.c: In function 'ehci_port_handed_over': drivers/usb/host/ehci-hub.c:1194:8: warning: array subscript '<unknown>' is outside the bounds of an interior zero-length array 'u32[0]' {aka 'unsigned int[]'} [-Wzero-length-bounds] 1194 | reg = &ehci->regs->port_status[portnum - 1]; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from drivers/usb/host/ehci.h:274, from drivers/usb/host/ehci-hcd.c:97: ./include/linux/usb/ehci_def.h:130:7: note: while referencing 'port_status' 130 | u32 port_status[0]; /* up to N_PORTS */ | ^~~~~~~~~~~ Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Al Cooper <alcooperx@gmail.com> Cc: Alan Stern <stern@rowland.harvard.edu> Cc: linux-usb@vger.kernel.org Acked-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20210818173018.2259231-2-keescook@chromium.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--include/linux/usb/ehci_def.h24
1 files changed, 13 insertions, 11 deletions
diff --git a/include/linux/usb/ehci_def.h b/include/linux/usb/ehci_def.h
index 78e006355557..dcbe2b068569 100644
--- a/include/linux/usb/ehci_def.h
+++ b/include/linux/usb/ehci_def.h
@@ -45,6 +45,7 @@ struct ehci_caps {
#define HCS_PORTROUTED(p) ((p)&(1 << 7)) /* true: port routing */
#define HCS_PPC(p) ((p)&(1 << 4)) /* true: port power control */
#define HCS_N_PORTS(p) (((p)>>0)&0xf) /* bits 3:0, ports on HC */
+#define HCS_N_PORTS_MAX 15 /* N_PORTS valid 0x1-0xF */
u32 hcc_params; /* HCCPARAMS - offset 0x8 */
/* EHCI 1.1 addendum */
@@ -126,8 +127,9 @@ struct ehci_regs {
u32 configured_flag;
#define FLAG_CF (1<<0) /* true: we'll support "high speed" */
- /* PORTSC: offset 0x44 */
- u32 port_status[0]; /* up to N_PORTS */
+ union {
+ /* PORTSC: offset 0x44 */
+ u32 port_status[HCS_N_PORTS_MAX]; /* up to N_PORTS */
/* EHCI 1.1 addendum */
#define PORTSC_SUSPEND_STS_ACK 0
#define PORTSC_SUSPEND_STS_NYET 1
@@ -164,28 +166,28 @@ struct ehci_regs {
#define PORT_CSC (1<<1) /* connect status change */
#define PORT_CONNECT (1<<0) /* device connected */
#define PORT_RWC_BITS (PORT_CSC | PORT_PEC | PORT_OCC)
-
- u32 reserved3[9];
-
- /* USBMODE: offset 0x68 */
- u32 usbmode; /* USB Device mode */
+ struct {
+ u32 reserved3[9];
+ /* USBMODE: offset 0x68 */
+ u32 usbmode; /* USB Device mode */
+ };
#define USBMODE_SDIS (1<<3) /* Stream disable */
#define USBMODE_BE (1<<2) /* BE/LE endianness select */
#define USBMODE_CM_HC (3<<0) /* host controller mode */
#define USBMODE_CM_IDLE (0<<0) /* idle state */
-
- u32 reserved4[6];
+ };
+ u32 reserved4;
/* Moorestown has some non-standard registers, partially due to the fact that
* its EHCI controller has both TT and LPM support. HOSTPCx are extensions to
* PORTSCx
*/
/* HOSTPC: offset 0x84 */
- u32 hostpc[0]; /* HOSTPC extension */
+ u32 hostpc[HCS_N_PORTS_MAX];
#define HOSTPC_PHCD (1<<22) /* Phy clock disable */
#define HOSTPC_PSPD (3<<25) /* Port speed detection */
- u32 reserved5[17];
+ u32 reserved5[2];
/* USBMODE_EX: offset 0xc8 */
u32 usbmode_ex; /* USB Device mode extension */