On Mon, 22 Apr 2024, Mark Cave-Ayland wrote:
When compiling with gcc 12 drivers/usb.c generates the following error:
/root/drivers/usb.c: In function 'get_descriptor': /root/drivers/usb.c:200:23: error: array subscript 'device_descriptor_t[0]' is partly outside array bounds of 'u8[8]' {aka 'unsigned char[8]'} [-Werror=array-bounds] 200 | if (dd->bMaxPacketSize0 != 0) | ^~ /root/drivers/usb.c:181:12: note: object 'buf' of size 8 181 | u8 buf[8]; | ^~~ cc1: all warnings being treated as errors make[1]: *** [rules.mak:229: target/drivers/usb.o] Error 1 make[1]: Leaving directory '/root/obj-ppc'
The compiler is complaining that a device_descriptor_t pointer cast to the 8 byte buffer means that a pointer dereference to some members of the device_descriptor_t could go beyond the end of the 8 byte buffer. Whilst this sounds as if it should be allowed, some searching suggests that the behaviour of such a cast is undefined.
Rework get_descriptor() so that an entire device_descriptor_t is used to store the first 8 bytes of the incoming descriptor, and update the buf references to use the equivalent device_descriptor_t fields instead.
Originally this came from libpayload of coreboot. To avoid diverging from that, aybe you can consider checking the changes there and updating this file from the current version of libpayload to also get the fixes and enhancements made there. This function seems to have been replaced in coreboot and not there any more.
Regards, BALATON Zoltan
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk
drivers/usb.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/usb.c b/drivers/usb.c index 88b7580..d0e1ae5 100644 --- a/drivers/usb.c +++ b/drivers/usb.c @@ -178,7 +178,7 @@ u8 * get_descriptor (usbdev_t *dev, unsigned char bmRequestType, int descType, int descIdx, int langID) {
- u8 buf[8];
- device_descriptor_t dd; u8 *result; dev_req_t dr; int size;
@@ -189,24 +189,23 @@ get_descriptor (usbdev_t *dev, unsigned char bmRequestType, int descType, dr.wValue = __cpu_to_le16((descType << 8) | descIdx); dr.wIndex = __cpu_to_le16(langID); dr.wLength = __cpu_to_le16(8);
- if (dev->controller->control (dev, IN, sizeof (dr), &dr, 8, buf)) {
if (dev->controller->control (dev, IN, sizeof (dr), &dr, 8, (u8 *)&dd)) { usb_debug ("getting descriptor size (type %x) failed\n", descType); }
if (descType == 1) {
device_descriptor_t *dd = (device_descriptor_t *) buf;usb_debug ("maxPacketSize0: %x\n", dd->bMaxPacketSize0);if (dd->bMaxPacketSize0 != 0)dev->endpoints[0].maxpacketsize = dd->bMaxPacketSize0;
usb_debug ("maxPacketSize0: %x\n", dd.bMaxPacketSize0);if (dd.bMaxPacketSize0 != 0)dev->endpoints[0].maxpacketsize = dd.bMaxPacketSize0;}
/* special case for configuration descriptors: they carry all their subsequent descriptors with them, and keep the entire size at a different location */
- size = buf[0];
- if (buf[1] == 2) {
int realsize = __le16_to_cpu(((unsigned short *) (buf + 2))[0]);
- size = dd.bLength;
- if (dd.bDescriptorType == 2) {
int realsize = __le16_to_cpu(dd.bcdUSB);