| Message ID | f0b62f38c042d2dcb8b8e83c827d76db2ac5d7ad.camel@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] net/usb: kalmia: Fix uninit-value in kalmia_send_init_packet | expand |
On Tue, Jan 31, 2023 at 03:20:33PM +0100, Miko Larsson wrote: > syzbot reports that act_len in kalmia_send_init_packet() is > uninitialized. Fix this by initializing it to 0. > > Fixes: d40261236e8e ("net/usb: Add Samsung Kalmia driver for Samsung GT-B3730") > Reported-and-tested-by: syzbot+cd80c5ef5121bfe85b55@syzkaller.appspotmail.com > Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Tue, Jan 31, 2023 at 03:20:33PM CET, mikoxyzzz@gmail.com wrote: >syzbot reports that act_len in kalmia_send_init_packet() is >uninitialized. Fix this by initializing it to 0. > >Fixes: d40261236e8e ("net/usb: Add Samsung Kalmia driver for Samsung GT-B3730") >Reported-and-tested-by: syzbot+cd80c5ef5121bfe85b55@syzkaller.appspotmail.com >Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com> >--- >v1 -> v2 >* Minor alteration of commit message. >* Added 'reported-and-tested-by' which is attributed to syzbot. > > drivers/net/usb/kalmia.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/drivers/net/usb/kalmia.c b/drivers/net/usb/kalmia.c >index 9f2b70ef39aa..b158fb7bf66a 100644 >--- a/drivers/net/usb/kalmia.c >+++ b/drivers/net/usb/kalmia.c >@@ -56,7 +56,7 @@ static int > kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 init_msg_len, > u8 *buffer, u8 expected_len) > { >- int act_len; >+ int act_len = 0; > int status; > > netdev_dbg(dev->net, "Sending init packet"); Hmm, this is not the right fix. If the second call of usb_bulk_msg() in this function returns != 0, the act_len printed out contains the value from previous usb_bulk_msg() call, which does not make sense. Printing act_len on error path is pointless, so rather remove it from the error message entirely for both usb_bulk_msg() calls.
On Wed, 2023-02-01 at 13:19 +0100, Jiri Pirko wrote: > Tue, Jan 31, 2023 at 03:20:33PM CET, mikoxyzzz@gmail.com wrote: > > syzbot reports that act_len in kalmia_send_init_packet() is > > uninitialized. Fix this by initializing it to 0. > > > > Fixes: d40261236e8e ("net/usb: Add Samsung Kalmia driver for > > Samsung GT-B3730") > > Reported-and-tested-by: > > syzbot+cd80c5ef5121bfe85b55@syzkaller.appspotmail.com > > Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com> > > --- > > v1 -> v2 > > * Minor alteration of commit message. > > * Added 'reported-and-tested-by' which is attributed to syzbot. > > > > drivers/net/usb/kalmia.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/usb/kalmia.c b/drivers/net/usb/kalmia.c > > index 9f2b70ef39aa..b158fb7bf66a 100644 > > --- a/drivers/net/usb/kalmia.c > > +++ b/drivers/net/usb/kalmia.c > > @@ -56,7 +56,7 @@ static int > > kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 > > init_msg_len, > > u8 *buffer, u8 expected_len) > > { > > - int act_len; > > + int act_len = 0; > > int status; > > > > netdev_dbg(dev->net, "Sending init packet"); > > Hmm, this is not the right fix. > > If the second call of usb_bulk_msg() in this function returns != 0, > the > act_len printed out contains the value from previous usb_bulk_msg() > call, > which does not make sense. > > Printing act_len on error path is pointless, so rather remove it from > the error message entirely for both usb_bulk_msg() calls. Something like this, then? diff --git a/drivers/net/usb/kalmia.c b/drivers/net/usb/kalmia.c index 9f2b70ef39aa..613fc6910f14 100644 --- a/drivers/net/usb/kalmia.c +++ b/drivers/net/usb/kalmia.c @@ -65,8 +65,8 @@ kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 init_msg_len, init_msg, init_msg_len, &act_len, KALMIA_USB_TIMEOUT); if (status != 0) { netdev_err(dev->net, - "Error sending init packet. Status %i, length %i\n", - status, act_len); + "Error sending init packet. Status %i\n", + status); return status; } else if (act_len != init_msg_len) { @@ -83,8 +83,8 @@ kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 init_msg_len, if (status != 0) netdev_err(dev->net, - "Error receiving init result. Status %i, length %i\n", - status, act_len); + "Error receiving init result. Status %i\n", + status); else if (act_len != expected_len) netdev_err(dev->net, "Unexpected init result length: %i\n", act_len);
Thu, Feb 09, 2023 at 03:47:12PM CET, mikoxyzzz@gmail.com wrote: >On Wed, 2023-02-01 at 13:19 +0100, Jiri Pirko wrote: >> Tue, Jan 31, 2023 at 03:20:33PM CET, mikoxyzzz@gmail.com wrote: >> > syzbot reports that act_len in kalmia_send_init_packet() is >> > uninitialized. Fix this by initializing it to 0. >> > >> > Fixes: d40261236e8e ("net/usb: Add Samsung Kalmia driver for >> > Samsung GT-B3730") >> > Reported-and-tested-by: >> > syzbot+cd80c5ef5121bfe85b55@syzkaller.appspotmail.com >> > Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com> >> > --- >> > v1 -> v2 >> > * Minor alteration of commit message. >> > * Added 'reported-and-tested-by' which is attributed to syzbot. >> > >> > drivers/net/usb/kalmia.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/drivers/net/usb/kalmia.c b/drivers/net/usb/kalmia.c >> > index 9f2b70ef39aa..b158fb7bf66a 100644 >> > --- a/drivers/net/usb/kalmia.c >> > +++ b/drivers/net/usb/kalmia.c >> > @@ -56,7 +56,7 @@ static int >> > kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 >> > init_msg_len, >> > u8 *buffer, u8 expected_len) >> > { >> > - int act_len; >> > + int act_len = 0; >> > int status; >> > >> > netdev_dbg(dev->net, "Sending init packet"); >> >> Hmm, this is not the right fix. >> >> If the second call of usb_bulk_msg() in this function returns != 0, >> the >> act_len printed out contains the value from previous usb_bulk_msg() >> call, >> which does not make sense. >> >> Printing act_len on error path is pointless, so rather remove it from >> the error message entirely for both usb_bulk_msg() calls. > >Something like this, then? Yes. > >diff --git a/drivers/net/usb/kalmia.c b/drivers/net/usb/kalmia.c >index 9f2b70ef39aa..613fc6910f14 100644 >--- a/drivers/net/usb/kalmia.c >+++ b/drivers/net/usb/kalmia.c >@@ -65,8 +65,8 @@ kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 init_msg_len, > init_msg, init_msg_len, &act_len, KALMIA_USB_TIMEOUT); > if (status != 0) { > netdev_err(dev->net, >- "Error sending init packet. Status %i, length %i\n", >- status, act_len); >+ "Error sending init packet. Status %i\n", >+ status); > return status; > } > else if (act_len != init_msg_len) { >@@ -83,8 +83,8 @@ kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 init_msg_len, > > if (status != 0) > netdev_err(dev->net, >- "Error receiving init result. Status %i, length %i\n", >- status, act_len); >+ "Error receiving init result. Status %i\n", >+ status); > else if (act_len != expected_len) > netdev_err(dev->net, "Unexpected init result length: %i\n", > act_len); > >-- >~miko
diff --git a/drivers/net/usb/kalmia.c b/drivers/net/usb/kalmia.c index 9f2b70ef39aa..b158fb7bf66a 100644 --- a/drivers/net/usb/kalmia.c +++ b/drivers/net/usb/kalmia.c @@ -56,7 +56,7 @@ static int kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 init_msg_len, u8 *buffer, u8 expected_len) { - int act_len; + int act_len = 0; int status; netdev_dbg(dev->net, "Sending init packet");
syzbot reports that act_len in kalmia_send_init_packet() is uninitialized. Fix this by initializing it to 0. Fixes: d40261236e8e ("net/usb: Add Samsung Kalmia driver for Samsung GT-B3730") Reported-and-tested-by: syzbot+cd80c5ef5121bfe85b55@syzkaller.appspotmail.com Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com> --- v1 -> v2 * Minor alteration of commit message. * Added 'reported-and-tested-by' which is attributed to syzbot. drivers/net/usb/kalmia.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)