| Message ID | 20191107004404.23707-1-tranmanphong@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: asix: Fix uninit-value in asix_mdio_write | expand |
From: Phong Tran <tranmanphong@gmail.com> Date: Thu, 7 Nov 2019 07:44:04 +0700 > The local variables use without initilization value. > This fixes the syzbot report. > > Reported-by: syzbot+7dc7c28d4577bbe55b10@syzkaller.appspotmail.com > > Test result: > > https://groups.google.com/d/msg/syzkaller-bugs/3H_n05x_sPU/sUoHhxgAAgAJ > > Signed-off-by: Phong Tran <tranmanphong@gmail.com> There are several more situations in this file where the data blob passed to asix_read_cmd() is read without pre-initialization not checking the return value from asix_read_cmd(). So, syzbot can see some of them but not all of them, yet all of them are buggy and should be fixed. These kinds of patches drive me absolutely crazy :-) Really, one of two things needs to happen, either asix_read_cmd() clears the incoming buffer unconditionally, or these call sites strictly must check the return value always before accessing the buffer after the call. I'm not applying this, sorry.
On 11/8/19 6:21 AM, David Miller wrote: > From: Phong Tran <tranmanphong@gmail.com> > Date: Thu, 7 Nov 2019 07:44:04 +0700 > >> The local variables use without initilization value. >> This fixes the syzbot report. >> >> Reported-by: syzbot+7dc7c28d4577bbe55b10@syzkaller.appspotmail.com >> >> Test result: >> >> https://groups.google.com/d/msg/syzkaller-bugs/3H_n05x_sPU/sUoHhxgAAgAJ >> >> Signed-off-by: Phong Tran <tranmanphong@gmail.com> > > There are several more situations in this file where the data blob passed > to asix_read_cmd() is read without pre-initialization not checking the > return value from asix_read_cmd(). > > So, syzbot can see some of them but not all of them, yet all of them > are buggy and should be fixed. > > These kinds of patches drive me absolutely crazy :-) > > Really, one of two things needs to happen, either asix_read_cmd() clears > the incoming buffer unconditionally, thank you for your suggestion. Sent Patch v2 reply-to this mail thread. regards, Phong.
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c index e39f41efda3e..3c7ccd8a9da8 100644 --- a/drivers/net/usb/asix_common.c +++ b/drivers/net/usb/asix_common.c @@ -444,8 +444,8 @@ void asix_set_multicast(struct net_device *net) int asix_mdio_read(struct net_device *netdev, int phy_id, int loc) { struct usbnet *dev = netdev_priv(netdev); - __le16 res; - u8 smsr; + __le16 res = 0; + u8 smsr = 0; int i = 0; int ret; @@ -478,7 +478,7 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val) { struct usbnet *dev = netdev_priv(netdev); __le16 res = cpu_to_le16(val); - u8 smsr; + u8 smsr = 0; int i = 0; int ret; @@ -508,8 +508,8 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val) int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc) { struct usbnet *dev = netdev_priv(netdev); - __le16 res; - u8 smsr; + __le16 res = 0; + u8 smsr = 0; int i = 0; int ret; @@ -543,7 +543,7 @@ asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc, int val) { struct usbnet *dev = netdev_priv(netdev); __le16 res = cpu_to_le16(val); - u8 smsr; + u8 smsr = 0; int i = 0; int ret;
The local variables use without initilization value. This fixes the syzbot report. Reported-by: syzbot+7dc7c28d4577bbe55b10@syzkaller.appspotmail.com Test result: https://groups.google.com/d/msg/syzkaller-bugs/3H_n05x_sPU/sUoHhxgAAgAJ Signed-off-by: Phong Tran <tranmanphong@gmail.com> --- drivers/net/usb/asix_common.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)