Thread (12 messages) flat view 12 messages, 4 authors, 2021-08-18

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

From: Pavel Skripkin <hidden>
Date: 2021-08-14 16:20:35
Also in: lkml, netdev

On 8/14/21 6:36 PM, Andrew Lunn wrote:
On Sat, Aug 14, 2021 at 04:55:05PM +0300, Pavel Skripkin wrote:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help