lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 12 May 2020 17:04:10 +0200
From:   Jerome Pouiller <Jerome.Pouiller@...abs.com>
To:     devel@...verdev.osuosl.org, linux-wireless@...r.kernel.org
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jérôme Pouiller 
        <jerome.pouiller@...abs.com>
Subject: [PATCH v2 13/17] staging: wfx: fix access to le32 attribute 'len'

From: Jérôme Pouiller <jerome.pouiller@...abs.com>

Sparse complains about the accesses to the field 'len' from struct hif_msg:

    drivers/staging/wfx/bh.c:88:32: warning: restricted __le16 degrades to integer
    drivers/staging/wfx/bh.c:88:32: warning: restricted __le16 degrades to integer
    drivers/staging/wfx/bh.c:93:32: warning: restricted __le16 degrades to integer
    drivers/staging/wfx/bh.c:93:32: warning: cast to restricted __le16
    drivers/staging/wfx/bh.c:93:32: warning: restricted __le16 degrades to integer
    drivers/staging/wfx/bh.c:121:25: warning: incorrect type in argument 2 (different base types)
    drivers/staging/wfx/bh.c:121:25:    expected unsigned int len
    drivers/staging/wfx/bh.c:121:25:    got restricted __le16 [usertype] len
    drivers/staging/wfx/hif_rx.c:27:22: warning: restricted __le16 degrades to integer
    drivers/staging/wfx/hif_rx.c:347:39: warning: incorrect type in argument 7 (different base types)
    drivers/staging/wfx/hif_rx.c:347:39:    expected unsigned int [usertype] len
    drivers/staging/wfx/hif_rx.c:347:39:    got restricted __le16 const [usertype] len
    drivers/staging/wfx/hif_rx.c:365:39: warning: incorrect type in argument 7 (different base types)
    drivers/staging/wfx/hif_rx.c:365:39:    expected unsigned int [usertype] len
    drivers/staging/wfx/hif_rx.c:365:39:    got restricted __le16 const [usertype] len
    drivers/staging/wfx/./traces.h:195:1: warning: incorrect type in assignment (different base types)
    drivers/staging/wfx/./traces.h:195:1:    expected int msg_len
    drivers/staging/wfx/./traces.h:195:1:    got restricted __le16 const [usertype] len
    drivers/staging/wfx/./traces.h:195:1: warning: incorrect type in assignment (different base types)
    drivers/staging/wfx/./traces.h:195:1:    expected int msg_len
    drivers/staging/wfx/./traces.h:195:1:    got restricted __le16 const [usertype] len
    drivers/staging/wfx/debug.c:319:20: warning: restricted __le16 degrades to integer
    drivers/staging/wfx/secure_link.c:85:27: warning: restricted __le16 degrades to integer
    drivers/staging/wfx/secure_link.c:85:27: warning: restricted __le16 degrades to integer

Indeed, the attribute len is little-endian. We have to take to the
endianness when we access it.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@...abs.com>
---
 drivers/staging/wfx/bh.c     | 13 ++++++-------
 drivers/staging/wfx/debug.c  |  2 +-
 drivers/staging/wfx/hif_rx.c |  6 +++---
 drivers/staging/wfx/traces.h |  2 +-
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 55724e4295c4..6c6e29cb7dcf 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -84,13 +84,12 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 			// piggyback is probably correct.
 			return piggyback;
 		}
-		le16_to_cpus(&hif->len);
-		computed_len = round_up(hif->len - sizeof(hif->len), 16)
-			       + sizeof(struct hif_sl_msg)
-			       + sizeof(struct hif_sl_tag);
+		computed_len =
+			round_up(le16_to_cpu(hif->len) - sizeof(hif->len), 16) +
+			sizeof(struct hif_sl_msg) +
+			sizeof(struct hif_sl_tag);
 	} else {
-		le16_to_cpus(&hif->len);
-		computed_len = round_up(hif->len, 2);
+		computed_len = round_up(le16_to_cpu(hif->len), 2);
 	}
 	if (computed_len != read_len) {
 		dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n",
@@ -118,7 +117,7 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 		wdev->hif.rx_seqnum = (hif->seqnum + 1) % (HIF_COUNTER_MAX + 1);
 	}
 
-	skb_put(skb, hif->len);
+	skb_put(skb, le16_to_cpu(hif->len));
 	// wfx_handle_rx takes care on SKB livetime
 	wfx_handle_rx(wdev, skb);
 	if (!wdev->hif.tx_buffers_used)
diff --git a/drivers/staging/wfx/debug.c b/drivers/staging/wfx/debug.c
index 846a0b29f8c9..f52e7cf885cb 100644
--- a/drivers/staging/wfx/debug.c
+++ b/drivers/staging/wfx/debug.c
@@ -250,7 +250,7 @@ static ssize_t wfx_send_hif_msg_write(struct file *file,
 	request = memdup_user(user_buf, count);
 	if (IS_ERR(request))
 		return PTR_ERR(request);
-	if (request->len != count) {
+	if (le16_to_cpu(request->len) != count) {
 		kfree(request);
 		return -EINVAL;
 	}
diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index 9b4f0c4ba745..36132909a6ae 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -24,7 +24,7 @@ static int hif_generic_confirm(struct wfx_dev *wdev,
 	// All confirm messages start with status
 	int status = le32_to_cpup((__le32 *)buf);
 	int cmd = hif->id;
-	int len = hif->len - 4; // drop header
+	int len = le16_to_cpu(hif->len) - 4; // drop header
 
 	WARN(!mutex_is_locked(&wdev->hif_cmd.lock), "data locking error");
 
@@ -348,7 +348,7 @@ static int hif_error_indication(struct wfx_dev *wdev,
 	else
 		dev_err(wdev->dev, "asynchronous error: unknown: %08x\n", type);
 	print_hex_dump(KERN_INFO, "hif: ", DUMP_PREFIX_OFFSET,
-		       16, 1, hif, hif->len, false);
+		       16, 1, hif, le16_to_cpu(hif->len), false);
 	wdev->chip_frozen = true;
 
 	return 0;
@@ -366,7 +366,7 @@ static int hif_exception_indication(struct wfx_dev *wdev,
 	else
 		dev_err(wdev->dev, "firmware exception\n");
 	print_hex_dump(KERN_INFO, "hif: ", DUMP_PREFIX_OFFSET,
-		       16, 1, hif, hif->len, false);
+		       16, 1, hif, le16_to_cpu(hif->len), false);
 	wdev->chip_frozen = true;
 
 	return -1;
diff --git a/drivers/staging/wfx/traces.h b/drivers/staging/wfx/traces.h
index 959a0d31bf4e..7298fb948f56 100644
--- a/drivers/staging/wfx/traces.h
+++ b/drivers/staging/wfx/traces.h
@@ -174,7 +174,7 @@ DECLARE_EVENT_CLASS(hif_data,
 		int header_len;
 
 		__entry->tx_fill_level = tx_fill_level;
-		__entry->msg_len = hif->len;
+		__entry->msg_len = le16_to_cpu(hif->len);
 		__entry->msg_id = hif->id;
 		__entry->if_id = hif->interface;
 		if (is_recv)
-- 
2.26.2

Powered by blists - more mailing lists