| 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
| ||
|
Message-Id: <20210818173018.2259231-2-keescook@chromium.org>
Date: Wed, 18 Aug 2021 10:30:17 -0700
From: Kees Cook <keescook@...omium.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Kees Cook <keescook@...omium.org>, Arnd Bergmann <arnd@...db.de>,
Al Cooper <alcooperx@...il.com>,
Alan Stern <stern@...land.harvard.edu>,
linux-usb@...r.kernel.org,
Andy Shevchenko <andy.shevchenko@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
linux-kernel@...r.kernel.org,
bcm-kernel-feedback-list@...adcom.com,
linux-hardening@...r.kernel.org
Subject: [PATCH v2 1/2] 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@...uxfoundation.org>
Cc: Arnd Bergmann <arnd@...db.de>
Cc: Al Cooper <alcooperx@...il.com>
Cc: Alan Stern <stern@...land.harvard.edu>
Cc: linux-usb@...r.kernel.org
Signed-off-by: Kees Cook <keescook@...omium.org>
---
include/linux/usb/ehci_def.h | 24 +++++++++++++-----------
1 file 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 */
--
2.30.2
Powered by blists - more mailing lists