netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Skripkin <paskripkin@gmail.com>
To: stefan@datenfreihafen.org, alex.aring@gmail.com,
	davem@davemloft.net, kuba@kernel.org
Cc: linux-wpan@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Pavel Skripkin <paskripkin@gmail.com>,
	stable@vger.kernel.org, Alexander Potapenko <glider@google.com>
Subject: [PATCH RFT] ieee802154: atusb: move to new USB API
Date: Sun,  2 Jan 2022 20:19:43 +0300	[thread overview]
Message-ID: <20220102171943.28846-1-paskripkin@gmail.com> (raw)
In-Reply-To: <CAG_fn=VDEoQx5c7XzWX1yaYBd5y5FrG1aagrkv+SZ03c8TfQYQ@mail.gmail.com>

Alexander reported a use of uninitialized value in
atusb_set_extended_addr(), that is caused by reading 0 bytes via
usb_control_msg().

Since there is an API, that cannot read less bytes, than was requested,
let's move atusb driver to use it. It will fix all potintial bugs with
uninit values and make code more modern

Fail log:

BUG: KASAN: uninit-cmp in ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
BUG: KASAN: uninit-cmp in atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
BUG: KASAN: uninit-cmp in atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
Uninit value used in comparison: 311daa649a2003bd stack handle: 000000009a2003bd
 ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
 atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
 atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
 usb_probe_interface+0x314/0x7f0 drivers/usb/core/driver.c:396

Fixes: 7490b008d123 ("ieee802154: add support for atusb transceiver")
Cc: stable@vger.kernel.org # 5.9
Reported-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/net/ieee802154/atusb.c | 61 +++++++++++++++++++++-------------
 1 file changed, 38 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index 23ee0b14cbfa..43befea0110f 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -80,10 +80,9 @@ struct atusb_chip_data {
  * in atusb->err and reject all subsequent requests until the error is cleared.
  */
 
-static int atusb_control_msg(struct atusb *atusb, unsigned int pipe,
-			     __u8 request, __u8 requesttype,
-			     __u16 value, __u16 index,
-			     void *data, __u16 size, int timeout)
+static int atusb_control_msg_recv(struct atusb *atusb, __u8 request, __u8 requesttype,
+				  __u16 value, __u16 index,
+				  void *data, __u16 size, int timeout)
 {
 	struct usb_device *usb_dev = atusb->usb_dev;
 	int ret;
@@ -91,8 +90,30 @@ static int atusb_control_msg(struct atusb *atusb, unsigned int pipe,
 	if (atusb->err)
 		return atusb->err;
 
-	ret = usb_control_msg(usb_dev, pipe, request, requesttype,
-			      value, index, data, size, timeout);
+	ret = usb_control_msg_recv(usb_dev, 0, request, requesttype,
+				   value, index, data, size, timeout, GFP_KERNEL);
+	if (ret < 0) {
+		atusb->err = ret;
+		dev_err(&usb_dev->dev,
+			"%s: req 0x%02x val 0x%x idx 0x%x, error %d\n",
+			__func__, request, value, index, ret);
+	}
+
+	return ret;
+}
+
+static int atusb_control_msg_send(struct atusb *atusb, __u8 request, __u8 requesttype,
+				  __u16 value, __u16 index,
+				  void *data, __u16 size, int timeout)
+{
+	struct usb_device *usb_dev = atusb->usb_dev;
+	int ret;
+
+	if (atusb->err)
+		return atusb->err;
+
+	ret = usb_control_msg_send(usb_dev, 0, request, requesttype,
+				   value, index, data, size, timeout, GFP_KERNEL);
 	if (ret < 0) {
 		atusb->err = ret;
 		dev_err(&usb_dev->dev,
@@ -107,8 +128,7 @@ static int atusb_command(struct atusb *atusb, u8 cmd, u8 arg)
 	struct usb_device *usb_dev = atusb->usb_dev;
 
 	dev_dbg(&usb_dev->dev, "%s: cmd = 0x%x\n", __func__, cmd);
-	return atusb_control_msg(atusb, usb_sndctrlpipe(usb_dev, 0),
-				 cmd, ATUSB_REQ_TO_DEV, arg, 0, NULL, 0, 1000);
+	return atusb_control_msg_send(atusb, cmd, ATUSB_REQ_TO_DEV, arg, 0, NULL, 0, 1000);
 }
 
 static int atusb_write_reg(struct atusb *atusb, u8 reg, u8 value)
@@ -116,9 +136,8 @@ static int atusb_write_reg(struct atusb *atusb, u8 reg, u8 value)
 	struct usb_device *usb_dev = atusb->usb_dev;
 
 	dev_dbg(&usb_dev->dev, "%s: 0x%02x <- 0x%02x\n", __func__, reg, value);
-	return atusb_control_msg(atusb, usb_sndctrlpipe(usb_dev, 0),
-				 ATUSB_REG_WRITE, ATUSB_REQ_TO_DEV,
-				 value, reg, NULL, 0, 1000);
+	return atusb_control_msg_send(atusb, ATUSB_REG_WRITE, ATUSB_REQ_TO_DEV,
+				      value, reg, NULL, 0, 1000);
 }
 
 static int atusb_read_reg(struct atusb *atusb, u8 reg)
@@ -133,9 +152,8 @@ static int atusb_read_reg(struct atusb *atusb, u8 reg)
 		return -ENOMEM;
 
 	dev_dbg(&usb_dev->dev, "%s: reg = 0x%x\n", __func__, reg);
-	ret = atusb_control_msg(atusb, usb_rcvctrlpipe(usb_dev, 0),
-				ATUSB_REG_READ, ATUSB_REQ_FROM_DEV,
-				0, reg, buffer, 1, 1000);
+	ret = atusb_control_msg_recv(atusb, ATUSB_REG_READ, ATUSB_REQ_FROM_DEV,
+				     0, reg, buffer, 1, 1000);
 
 	if (ret >= 0) {
 		value = buffer[0];
@@ -805,9 +823,8 @@ static int atusb_get_and_show_revision(struct atusb *atusb)
 		return -ENOMEM;
 
 	/* Get a couple of the ATMega Firmware values */
-	ret = atusb_control_msg(atusb, usb_rcvctrlpipe(usb_dev, 0),
-				ATUSB_ID, ATUSB_REQ_FROM_DEV, 0, 0,
-				buffer, 3, 1000);
+	ret = atusb_control_msg_recv(atusb, ATUSB_ID, ATUSB_REQ_FROM_DEV, 0, 0,
+				     buffer, 3, 1000);
 	if (ret >= 0) {
 		atusb->fw_ver_maj = buffer[0];
 		atusb->fw_ver_min = buffer[1];
@@ -861,9 +878,8 @@ static int atusb_get_and_show_build(struct atusb *atusb)
 	if (!build)
 		return -ENOMEM;
 
-	ret = atusb_control_msg(atusb, usb_rcvctrlpipe(usb_dev, 0),
-				ATUSB_BUILD, ATUSB_REQ_FROM_DEV, 0, 0,
-				build, ATUSB_BUILD_SIZE, 1000);
+	ret = atusb_control_msg_recv(atusb, ATUSB_BUILD, ATUSB_REQ_FROM_DEV, 0, 0,
+				     build, ATUSB_BUILD_SIZE, 1000);
 	if (ret >= 0) {
 		build[ret] = 0;
 		dev_info(&usb_dev->dev, "Firmware: build %s\n", build);
@@ -985,9 +1001,8 @@ static int atusb_set_extended_addr(struct atusb *atusb)
 		return -ENOMEM;
 
 	/* Firmware is new enough so we fetch the address from EEPROM */
-	ret = atusb_control_msg(atusb, usb_rcvctrlpipe(usb_dev, 0),
-				ATUSB_EUI64_READ, ATUSB_REQ_FROM_DEV, 0, 0,
-				buffer, IEEE802154_EXTENDED_ADDR_LEN, 1000);
+	ret = atusb_control_msg_recv(atusb, ATUSB_EUI64_READ, ATUSB_REQ_FROM_DEV, 0, 0,
+				     buffer, IEEE802154_EXTENDED_ADDR_LEN, 1000);
 	if (ret < 0) {
 		dev_err(&usb_dev->dev, "failed to fetch extended address, random address set\n");
 		ieee802154_random_extended_addr(&atusb->hw->phy->perm_extended_addr);
-- 
2.34.1


       reply	other threads:[~2022-01-02 17:20 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAG_fn=VDEoQx5c7XzWX1yaYBd5y5FrG1aagrkv+SZ03c8TfQYQ@mail.gmail.com>
2022-01-02 17:19 ` Pavel Skripkin [this message]
2022-01-02 22:15   ` [PATCH RFT] ieee802154: atusb: move to new USB API Alexander Aring
2022-01-02 22:21     ` Pavel Skripkin
2022-01-02 22:36       ` Alexander Aring
2022-01-03 12:09         ` [PATCH v2] ieee802154: atusb: fix uninit value in atusb_set_extended_addr Pavel Skripkin
2022-01-03 15:27           ` Alexander Aring
2022-01-04 15:40           ` Stefan Schmidt
2022-01-04 17:27             ` Pavel Skripkin
2022-01-04 18:04               ` Stefan Schmidt
2022-01-04 18:11                 ` Pavel Skripkin
2022-01-04 18:28                 ` [PATCH v3] " Pavel Skripkin
2022-01-04 18:57                   ` Alexander Aring
2022-01-04 19:25                     ` Stefan Schmidt
2022-01-03 13:04     ` [PATCH RFT] ieee802154: atusb: move to new USB API Greg KH
2022-01-03 13:03   ` Greg KH
2022-01-03 15:35     ` Alexander Aring
2022-01-04 19:41       ` Stefan Schmidt
2022-01-05  8:08         ` Greg KH
2022-01-05  9:01           ` Stefan Schmidt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220102171943.28846-1-paskripkin@gmail.com \
    --to=paskripkin@gmail.com \
    --cc=alex.aring@gmail.com \
    --cc=davem@davemloft.net \
    --cc=glider@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wpan@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=stefan@datenfreihafen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).