From: Pavel Skripkin <hidden> Date: 2021-08-13 16:01:19
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 cheking and just continue the loop instead of
checking uninit value.
Fixes: 8a46f665833a ("net: asix: Avoid looping when the device is disconnected")
Reported-and-tested-by: syzbot+a631ec9e717fb0423053@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <redacted>
---
drivers/net/usb/asix_common.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
@@ -479,7 +479,13 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)usleep_range(1000,1100);ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,0,0,1,&smsr,0);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));+if(ret==-ENODEV){+break;+}elseif(ret<0){+++i;+continue;+}+}while(!(smsr&AX_HOST_EN)&&(i++<30));if(ret==-ENODEV||ret==-ETIMEDOUT){mutex_unlock(&dev->phy_mutex);returnret;
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-08-13 22:23:31
On Fri, Aug 13, 2021 at 07:01:08PM +0300, Pavel Skripkin wrote:
quoted hunk
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 cheking and just continue the loop instead of
checking uninit value.
Fixes: 8a46f665833a ("net: asix: Avoid looping when the device is disconnected")
Reported-and-tested-by: syzbot+a631ec9e717fb0423053@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <redacted>
---
drivers/net/usb/asix_common.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
@@ -479,7 +479,13 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)usleep_range(1000,1100);ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,0,0,1,&smsr,0);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));+if(ret==-ENODEV){+break;+}elseif(ret<0){+++i;+continue;+}+}while(!(smsr&AX_HOST_EN)&&(i++<30));
No ret < 0, don't you end up with a double increment of i? So it will
only retry 15 times, not 30?
Humm.
If ret < 0 is true, smsr is uninitialized? The continue statement
causes a jump into the condition expression, where we evaluate smsr &
AX_HOST_EN. Isn't this just as broken as the original version?
Andrew
From: Pavel Skripkin <hidden> Date: 2021-08-13 22:29:11
On 8/14/21 1:23 AM, Andrew Lunn wrote:
On Fri, Aug 13, 2021 at 07:01:08PM +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 cheking and just continue the loop instead of
checking uninit value.
Fixes: 8a46f665833a ("net: asix: Avoid looping when the device is disconnected")
Reported-and-tested-by: syzbot+a631ec9e717fb0423053@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <redacted>
---
drivers/net/usb/asix_common.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
@@ -479,7 +479,13 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)usleep_range(1000,1100);ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,0,0,1,&smsr,0);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));+if(ret==-ENODEV){+break;+}elseif(ret<0){+++i;+continue;+}+}while(!(smsr&AX_HOST_EN)&&(i++<30));
No ret < 0, don't you end up with a double increment of i? So it will
only retry 15 times, not 30?
Humm.
If ret < 0 is true, smsr is uninitialized? The continue statement
causes a jump into the condition expression, where we evaluate smsr &
AX_HOST_EN. Isn't this just as broken as the original version?
Andrew
Yes, you are right, I missed that, sorry. I will rewrote this loop into
for loop in v2.
Im wondering why this wrong patch passed KMSAN testing...
With regards,
Pavel Skripkin
From: Pavel Skripkin <hidden> Date: 2021-08-13 22:44:24
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 cheking and just continue the loop instead of
checking uninit value.
Fixes: 8a46f665833a ("net: asix: Avoid looping when the device is disconnected")
Reported-by: syzbot+a631ec9e717fb0423053@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <redacted>
---
Changes in v2:
1. Fixed previous wrong approach and changed while loop to for loop
2. Reported-and-tested-by: tag removed, since KMSAN tests can be
false positive. Used Reported-by instead.
---
drivers/net/usb/asix_common.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
@@ -468,18 +468,25 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)structusbnet*dev=netdev_priv(netdev);__le16res;u8smsr;-inti=0;+inti;intret;mutex_lock(&dev->phy_mutex);-do{+for(i=0;i<30;++i){ret=asix_set_sw_mii(dev,0);if(ret==-ENODEV||ret==-ETIMEDOUT)break;usleep_range(1000,1100);ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,0,0,1,&smsr,0);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));+if(ret==-ENODEV)+break;+elseif(ret<0)+continue;+elseif(smsr&AX_HOST_EN)+break;+}+if(ret==-ENODEV||ret==-ETIMEDOUT){mutex_unlock(&dev->phy_mutex);returnret;
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-08-13 22:52:32
On Sat, 14 Aug 2021 01:42:19 +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 cheking and just continue the loop instead of
checking uninit value.
Fixes: 8a46f665833a ("net: asix: Avoid looping when the device is disconnected")
This is not the right tag, the Fixes tag should point to the commit
where the problem is introduced. Robert/Vincent only added some error
checking, the issue was there before, right?
Once you locate the right starting point for the fix please make sure
to add the author to CC.
Thanks!
Reported-by: syzbot+a631ec9e717fb0423053@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <redacted>
@@ -468,18 +468,25 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)structusbnet*dev=netdev_priv(netdev);__le16res;u8smsr;-inti=0;+inti;intret;
nit: move i after ret, reverse xmas tree style
mutex_lock(&dev->phy_mutex);
- do {
+ for (i = 0; i < 30; ++i) {
ret = asix_set_sw_mii(dev, 0);
if (ret == -ENODEV || ret == -ETIMEDOUT)
break;
usleep_range(1000, 1100);
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
0, 0, 1, &smsr, 0);
- } while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
+ if (ret == -ENODEV)
+ break;
+ else if (ret < 0)
+ continue;
+ else if (smsr & AX_HOST_EN)
+ break;
+ }
+
if (ret == -ENODEV || ret == -ETIMEDOUT) {
mutex_unlock(&dev->phy_mutex);
return ret;
Code LGTM, do other functions which Robert/Vincent touched not need the
same treatment tho?
@@ -468,18 +468,25 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)structusbnet*dev=netdev_priv(netdev);__le16res;u8smsr;-inti=0;+inti;intret;mutex_lock(&dev->phy_mutex);-do{+for(i=0;i<30;++i){ret=asix_set_sw_mii(dev,0);if(ret==-ENODEV||ret==-ETIMEDOUT)break;usleep_range(1000,1100);ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,0,0,1,&smsr,0);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));+if(ret==-ENODEV)+break;+elseif(ret<0)+continue;+elseif(smsr&AX_HOST_EN)+break;+}+
Yes, this looks good. And Jakub is correct, there are 3 other bits of
similar code you should look at.
Andrew
From: Pavel Skripkin <hidden> Date: 2021-08-14 13:55:31
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.
Cc: Robert Foss <redacted>
Fixes: d9fe64e51114 ("net: asix: Add in_pm parameter")
Reported-by: syzbot+a631ec9e717fb0423053@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <redacted>
---
Changes in v2:
1. Fixed previous wrong approach and changed while loop to for loop
2. Reported-and-tested-by: tag removed, since KMSAN tests can be
false positive. Used Reported-by instead.
Changes in v3:
1. Addressed uninit value bugs in asix_mdio_write(), asix_mdio_read_nopm()
and asix_mdio_write_nopm()
2. Moved i after ret to reverse xmas tree style
3. Fixed Fixes: tag
---
drivers/net/usb/asix_common.c | 51 ++++++++++++++++++++++++++---------
1 file changed, 39 insertions(+), 12 deletions(-)
@@ -468,18 +468,25 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)structusbnet*dev=netdev_priv(netdev);__le16res;u8smsr;-inti=0;intret;+inti;mutex_lock(&dev->phy_mutex);-do{+for(i=0;i<30;++i){ret=asix_set_sw_mii(dev,0);if(ret==-ENODEV||ret==-ETIMEDOUT)break;usleep_range(1000,1100);ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,0,0,1,&smsr,0);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));+if(ret==-ENODEV)+break;+elseif(ret<0)+continue;+elseif(smsr&AX_HOST_EN)+break;+}+if(ret==-ENODEV||ret==-ETIMEDOUT){mutex_unlock(&dev->phy_mutex);returnret;
@@ -506,21 +513,27 @@ static int __asix_mdio_write(struct net_device *netdev, int phy_id, int loc,structusbnet*dev=netdev_priv(netdev);__le16res=cpu_to_le16(val);u8smsr;-inti=0;intret;+inti;netdev_dbg(dev->net,"asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n",phy_id,loc,val);mutex_lock(&dev->phy_mutex);-do{+for(i=0;i<30;++i){ret=asix_set_sw_mii(dev,0);if(ret==-ENODEV)break;usleep_range(1000,1100);ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,0,0,1,&smsr,0);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));+if(ret==-ENODEV)+break;+elseif(ret<0)+continue;+elseif(smsr&AX_HOST_EN)+break;+}if(ret==-ENODEV)gotoout;
@@ -562,18 +575,25 @@ int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)structusbnet*dev=netdev_priv(netdev);__le16res;u8smsr;-inti=0;intret;+inti;mutex_lock(&dev->phy_mutex);-do{+for(i=0;i<30;++i){ret=asix_set_sw_mii(dev,1);if(ret==-ENODEV||ret==-ETIMEDOUT)break;usleep_range(1000,1100);ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,0,0,1,&smsr,1);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));+if(ret==-ENODEV)+break;+elseif(ret<0)+continue;+elseif(smsr&AX_HOST_EN)+break;+}+if(ret==-ENODEV||ret==-ETIMEDOUT){mutex_unlock(&dev->phy_mutex);returnret;
@@ -596,21 +616,28 @@ asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc, int val)structusbnet*dev=netdev_priv(netdev);__le16res=cpu_to_le16(val);u8smsr;-inti=0;intret;+inti;netdev_dbg(dev->net,"asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n",phy_id,loc,val);mutex_lock(&dev->phy_mutex);-do{+for(i=0;i<30;++i){ret=asix_set_sw_mii(dev,1);if(ret==-ENODEV)break;usleep_range(1000,1100);ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,0,0,1,&smsr,1);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));+if(ret==-ENODEV)+break;+elseif(ret<0)+continue;+elseif(smsr&AX_HOST_EN)+break;+}+if(ret==-ENODEV){mutex_unlock(&dev->phy_mutex);return;
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-08-14 15:37:04
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
From: Pavel Skripkin <hidden> Date: 2021-08-14 15:40:31
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.
It makes sense. Will add a helper function in v4. Thank you for
suggestion and review!
From: Pavel Skripkin <hidden> Date: 2021-08-14 16:20:35
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
From: Pavel Skripkin <hidden> Date: 2021-08-17 16:37:35
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.
Added helper function for checking Host_En bit, since wrong loop was used
in 4 functions and there is no need in copy-pasting code parts.
Cc: Robert Foss <redacted>
Fixes: d9fe64e51114 ("net: asix: Add in_pm parameter")
Reported-by: syzbot+a631ec9e717fb0423053@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <redacted>
---
Changes in v2:
1. Fixed previous wrong approach and changed while loop to for loop
2. Reported-and-tested-by: tag removed, since KMSAN tests can be
false positive. Used Reported-by instead.
Changes in v3:
1. Addressed uninit value bugs in asix_mdio_write(), asix_mdio_read_nopm()
and asix_mdio_write_nopm()
2. Moved i after ret to reverse xmas tree style
3. Fixed Fixes: tag
Changes in v4:
1. Added helper for checking Host_En bit, since wrong loop was
used in 4 functions. (Suggested by Andrew Lunn)
---
drivers/net/usb/asix_common.c | 70 +++++++++++++++--------------------
1 file changed, 30 insertions(+), 40 deletions(-)
@@ -63,6 +63,29 @@ void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,value,index,data,size);}+staticintasix_check_host_enable(structusbnet*dev,intin_pm)+{+inti,ret;+u8smsr;++for(i=0;i<30;++i){+ret=asix_set_sw_mii(dev,in_pm);+if(ret==-ENODEV||ret==-ETIMEDOUT)+break;+usleep_range(1000,1100);+ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,+0,0,1,&smsr,in_pm);+if(ret==-ENODEV)+break;+elseif(ret<0)+continue;+elseif(smsr&AX_HOST_EN)+break;+}++returnret;+}+staticvoidreset_asix_rx_fixup_info(structasix_rx_fixup_info*rx){/* Reset the variables that have a lifetime outside of
@@ -467,19 +490,11 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc){structusbnet*dev=netdev_priv(netdev);__le16res;-u8smsr;-inti=0;intret;mutex_lock(&dev->phy_mutex);-do{-ret=asix_set_sw_mii(dev,0);-if(ret==-ENODEV||ret==-ETIMEDOUT)-break;-usleep_range(1000,1100);-ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,-0,0,1,&smsr,0);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));++ret=asix_check_host_enable(dev,0);if(ret==-ENODEV||ret==-ETIMEDOUT){mutex_unlock(&dev->phy_mutex);returnret;
@@ -505,23 +520,14 @@ static int __asix_mdio_write(struct net_device *netdev, int phy_id, int loc,{structusbnet*dev=netdev_priv(netdev);__le16res=cpu_to_le16(val);-u8smsr;-inti=0;intret;netdev_dbg(dev->net,"asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n",phy_id,loc,val);mutex_lock(&dev->phy_mutex);-do{-ret=asix_set_sw_mii(dev,0);-if(ret==-ENODEV)-break;-usleep_range(1000,1100);-ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,-0,0,1,&smsr,0);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));+ret=asix_check_host_enable(dev,0);if(ret==-ENODEV)gotoout;
@@ -561,19 +567,11 @@ int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc){structusbnet*dev=netdev_priv(netdev);__le16res;-u8smsr;-inti=0;intret;mutex_lock(&dev->phy_mutex);-do{-ret=asix_set_sw_mii(dev,1);-if(ret==-ENODEV||ret==-ETIMEDOUT)-break;-usleep_range(1000,1100);-ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,-0,0,1,&smsr,1);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));++ret=asix_check_host_enable(dev,1);if(ret==-ENODEV||ret==-ETIMEDOUT){mutex_unlock(&dev->phy_mutex);returnret;
@@ -595,22 +593,14 @@ asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc, int val){structusbnet*dev=netdev_priv(netdev);__le16res=cpu_to_le16(val);-u8smsr;-inti=0;intret;netdev_dbg(dev->net,"asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n",phy_id,loc,val);mutex_lock(&dev->phy_mutex);-do{-ret=asix_set_sw_mii(dev,1);-if(ret==-ENODEV)-break;-usleep_range(1000,1100);-ret=asix_read_cmd(dev,AX_CMD_STATMNGSTS_REG,-0,0,1,&smsr,1);-}while(!(smsr&AX_HOST_EN)&&(i++<30)&&(ret!=-ENODEV));++ret=asix_check_host_enable(dev,1);if(ret==-ENODEV){mutex_unlock(&dev->phy_mutex);return;
Hello:
This patch was applied to netdev/net.git (refs/heads/master):
On Tue, 17 Aug 2021 19:37:23 +0300 you 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.
[...]