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
