[PATCH] usb: c67x00: fix uninitialized data access

[PATCH] usb: c67x00: fix uninitialized data access

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Randconfig builds for s390 using gcc-10.5 revealed that the
c67x00_ll_husb_init_host_port() writes bogus data into the
registers:

drivers/usb/c67x00/c67x00-ll-hpi.c: In function 'c67x00_ll_husb_init_host_port':
drivers/usb/c67x00/c67x00-ll-hpi.c:288:3: error: '*(u16 *)((char *)&data+-452)' is used uninitialized in this function [-Werror=uninitialized]
  288 |   hpi_write_word(dev, COMM_R(i), data->regs[i]);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I have not seen this error before, but the compiler is clearly correct
and the bug has been in the driver since it was added in 2008.
This particular build has CONFIG_INIT_STACK_NONE=y and CONFIG_UBSAN=y,
but I don't think that alone is sufficient to find the bug.

Change this to write zeroes instead, which may still not be what
is intended but at least avoids the undefined behavior and the
warning about it.

Fixes: e9b29ffc519b ("USB: add Cypress c67x00 OTG controller HCD driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/usb/c67x00/c67x00-ll-hpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/c67x00/c67x00-ll-hpi.c b/drivers/usb/c67x00/c67x00-ll-hpi.c
index 7a214a3a6cc7..3825552dc024 100644
--- a/drivers/usb/c67x00/c67x00-ll-hpi.c
+++ b/drivers/usb/c67x00/c67x00-ll-hpi.c
@@ -306,7 +306,7 @@ void c67x00_ll_set_husb_eot(struct c67x00_device *dev, u16 value)
 static inline void c67x00_ll_husb_sie_init(struct c67x00_sie *sie)
 {
 	struct c67x00_device *dev = sie->dev;
-	struct c67x00_lcp_int_data data;
+	struct c67x00_lcp_int_data data = {};
 	int rc;
 
 	rc = c67x00_comm_exec_int(dev, HUSB_SIE_INIT_INT(sie->sie_num), &data);
-- 
2.53.0

Re: [PATCH] usb: c67x00: fix uninitialized data access

[Peter Korsgaard]

>>>>> "Arnd" == Arnd Bergmann <arnd@kernel.org> writes:

 > From: Arnd Bergmann <arnd@arndb.de>
 > Randconfig builds for s390 using gcc-10.5 revealed that the
 > c67x00_ll_husb_init_host_port() writes bogus data into the
 > registers:

 > drivers/usb/c67x00/c67x00-ll-hpi.c: In function 'c67x00_ll_husb_init_host_port':
 > drivers/usb/c67x00/c67x00-ll-hpi.c:288:3: error: '*(u16 *)((char *)&data+-452)' is used uninitialized in this function [-Werror=uninitialized]
 >   288 |   hpi_write_word(dev, COMM_R(i), data->regs[i]);
 >       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 > I have not seen this error before, but the compiler is clearly correct
 > and the bug has been in the driver since it was added in 2008.
 > This particular build has CONFIG_INIT_STACK_NONE=y and CONFIG_UBSAN=y,
 > but I don't think that alone is sufficient to find the bug.

 > Change this to write zeroes instead, which may still not be what
 > is intended but at least avoids the undefined behavior and the
 > warning about it.

Sorry, I don't have access to the documentation anymore :/


 > Fixes: e9b29ffc519b ("USB: add Cypress c67x00 OTG controller HCD driver")
 > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Peter Korsgaard <peter@korsgaard.com>

I wonder if we have any users anymore? I have myself not have access to
any platforms with this cypress controller for the last 10+ years or so,
so maybe the driver should just be dropped?


> ---
 >  drivers/usb/c67x00/c67x00-ll-hpi.c | 2 +-
 >  1 file changed, 1 insertion(+), 1 deletion(-)

 > diff --git a/drivers/usb/c67x00/c67x00-ll-hpi.c b/drivers/usb/c67x00/c67x00-ll-hpi.c
 > index 7a214a3a6cc7..3825552dc024 100644
 > --- a/drivers/usb/c67x00/c67x00-ll-hpi.c
 > +++ b/drivers/usb/c67x00/c67x00-ll-hpi.c
 > @@ -306,7 +306,7 @@ void c67x00_ll_set_husb_eot(struct c67x00_device *dev, u16 value)
 >  static inline void c67x00_ll_husb_sie_init(struct c67x00_sie *sie)
 >  {
 >  	struct c67x00_device *dev = sie->dev;
 > -	struct c67x00_lcp_int_data data;
 > +	struct c67x00_lcp_int_data data = {};
 >  	int rc;
 
 >  	rc = c67x00_comm_exec_int(dev, HUSB_SIE_INIT_INT(sie->sie_num), &data);
 > -- 

 > 2.53.0


-- 
Bye, Peter Korsgaard