Re: [PATCH v3] net: asix: fix uninit value bugs

[Pavel Skripkin]

On 8/14/21 6:36 PM, Andrew Lunn wrote:
> On Sat, Aug 14, 2021 at 04:55:05PM +0300, Pavel Skripkin wrote:
> > Syzbot reported uninit-value in asix_mdio_read(). The problem was in
> > missing error handling. asix_read_cmd() should initialize passed stack
> > variable smsr, but it can fail in some cases. Then while condidition
> > checks possibly uninit smsr variable.
> >
> > Since smsr is uninitialized stack variable, driver can misbehave,
> > because smsr will be random in case of asix_read_cmd() failure.
> > Fix it by adding error handling and just continue the loop instead of
> > checking uninit value.
> >
> > Also, same loop was used in 3 other functions. Fixed uninit value bug
> > in them too.
>
> Hi Pavel
>
> Which suggests it might make sense to refactor the code to make a
> helper? I will leave you to decide if you want to do that.
>
> The code does looks correct now.
>
> 	Andrew

I noticed strange thing. For example: driver looped 30 times, there 
wasn't any errors with usb transfer, but Host_En bit is not set. 
Datasheet says, that if Host_En is not set, that means software access 
will be ignored. Driver code doesn't handle this situation. We only 
check if ret is -ENODEV or -ETIMEOUT.

I guess, next register access will fail, but anyway, does it make sense 
to return when Host_En bit is not set?


With regards,
Pavel Skripkin