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

[Peter Korsgaard <peter@korsgaard.com>]

>>>>> "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