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] [day] [month] [year] [list]
Date:   Tue, 12 May 2020 17:04:11 +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 14/17] staging: wfx: fix endianness of the field 'status'

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

The field 'status' appears in most of structs returned by the hardware.
This field is encoded as little endian. Sparse complains this field is
not always correctly accessed:

    drivers/staging/wfx/data_rx.c:53:16: warning: restricted __le32 degrades to integer
    drivers/staging/wfx/data_rx.c:84:16: warning: restricted __le32 degrades to integer
    drivers/staging/wfx/data_tx.c:526:24: warning: restricted __le32 degrades to integer
    drivers/staging/wfx/data_tx.c:569:23: warning: restricted __le32 degrades to integer
    drivers/staging/wfx/hif_rx.c:128:33: warning: restricted __le32 degrades to integer
    drivers/staging/wfx/./traces.h:401:1: warning: restricted __le32 degrades to integer
    drivers/staging/wfx/./traces.h:401:1: warning: restricted __le32 degrades to integer

In most of cases, this field is only compared with HIF_STATUS values.
Finally, it is more convenient to solve the problem by defining the
HIF_STATUS values directly in little endian.

It is also the right time to make some clean up in the HIF_STATUS names.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@...abs.com>
---
 drivers/staging/wfx/data_rx.c         |  4 +--
 drivers/staging/wfx/data_tx.c         |  4 +--
 drivers/staging/wfx/hif_api_cmd.h     | 16 ------------
 drivers/staging/wfx/hif_api_general.h | 36 ++++++++++++++++-----------
 drivers/staging/wfx/hif_rx.c          |  2 +-
 drivers/staging/wfx/hif_tx.c          |  4 +--
 drivers/staging/wfx/main.c            |  2 +-
 drivers/staging/wfx/traces.h          |  2 +-
 8 files changed, 30 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c
index c3b3edae3420..0e959ebc38b5 100644
--- a/drivers/staging/wfx/data_rx.c
+++ b/drivers/staging/wfx/data_rx.c
@@ -49,7 +49,7 @@ static int wfx_drop_encrypt_data(struct wfx_dev *wdev,
 	}
 
 	/* Firmware strips ICV in case of MIC failure. */
-	if (arg->status == HIF_STATUS_MICFAILURE)
+	if (arg->status == HIF_STATUS_RX_FAIL_MIC)
 		icv_len = 0;
 
 	if (skb->len < hdrlen + iv_len + icv_len) {
@@ -79,7 +79,7 @@ void wfx_rx_cb(struct wfx_vif *wvif,
 	     ieee80211_is_beacon(frame->frame_control)))
 		goto drop;
 
-	if (arg->status == HIF_STATUS_MICFAILURE)
+	if (arg->status == HIF_STATUS_RX_FAIL_MIC)
 		hdr->flag |= RX_FLAG_MMIC_ERROR;
 	else if (arg->status)
 		goto drop;
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 014fa36c8f78..4a2910897b6f 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -528,7 +528,7 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 		if (rate->idx < 0)
 			break;
 		if (tx_count < rate->count &&
-		    arg->status == HIF_STATUS_RETRY_EXCEEDED &&
+		    arg->status == HIF_STATUS_TX_FAIL_RETRIES &&
 		    arg->ack_failures)
 			dev_dbg(wvif->wdev->dev,
 				"all retries were not consumed: %d != %d\n",
@@ -568,7 +568,7 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 			tx_info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED;
 		else
 			tx_info->flags |= IEEE80211_TX_STAT_ACK;
-	} else if (arg->status == HIF_REQUEUE) {
+	} else if (arg->status == HIF_STATUS_TX_FAIL_REQUEUE) {
 		WARN(!arg->tx_result_flags.requeue,
 		     "incoherent status and result_flags");
 		if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) {
diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index bb8c57291f74..d76722bff7ee 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -66,22 +66,6 @@ union hif_commands_ids {
 	enum hif_indications_ids indication;
 };
 
-enum hif_status {
-	HIF_STATUS_SUCCESS              = 0x0,
-	HIF_STATUS_FAILURE              = 0x1,
-	HIF_INVALID_PARAMETER           = 0x2,
-	HIF_STATUS_WARNING              = 0x3,
-	HIF_ERROR_UNSUPPORTED_MSG_ID    = 0x4,
-	HIF_STATUS_DECRYPTFAILURE       = 0x10,
-	HIF_STATUS_MICFAILURE           = 0x11,
-	HIF_STATUS_NO_KEY_FOUND         = 0x12,
-	HIF_STATUS_RETRY_EXCEEDED       = 0x13,
-	HIF_STATUS_TX_LIFETIME_EXCEEDED = 0x14,
-	HIF_REQUEUE                     = 0x15,
-	HIF_STATUS_REFUSED              = 0x16,
-	HIF_STATUS_BUSY                 = 0x17
-};
-
 struct hif_reset_flags {
 	u8     reset_stat:1;
 	u8     reset_all_int:1;
diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h
index 995752b9f168..f5abd8174706 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -67,21 +67,27 @@ enum hif_general_indications_ids {
 	HIF_IND_ID_SL_EXCHANGE_PUB_KEYS = 0xe5
 };
 
-enum hif_hi_status {
-	HI_STATUS_SUCCESS                             = 0x0000,
-	HI_STATUS_FAILURE                             = 0x0001,
-	HI_INVALID_PARAMETER                          = 0x0002,
-	HI_STATUS_GPIO_WARNING                        = 0x0003,
-	HI_ERROR_UNSUPPORTED_MSG_ID                   = 0x0004,
-	SL_MAC_KEY_STATUS_SUCCESS                     = 0x005A,
-	SL_MAC_KEY_STATUS_FAILED_KEY_ALREADY_BURNED   = 0x006B,
-	SL_MAC_KEY_STATUS_FAILED_RAM_MODE_NOT_ALLOWED = 0x007C,
-	SL_MAC_KEY_STATUS_FAILED_UNKNOWN_MODE         = 0x008D,
-	SL_PUB_KEY_EXCHANGE_STATUS_SUCCESS            = 0x009E,
-	SL_PUB_KEY_EXCHANGE_STATUS_FAILED             = 0x00AF,
-	PREVENT_ROLLBACK_CNF_SUCCESS                  = 0x1234,
-	PREVENT_ROLLBACK_CNF_WRONG_MAGIC_WORD         = 0x1256
-};
+#define HIF_STATUS_SUCCESS                         (cpu_to_le32(0x0000))
+#define HIF_STATUS_FAIL                            (cpu_to_le32(0x0001))
+#define HIF_STATUS_INVALID_PARAMETER               (cpu_to_le32(0x0002))
+#define HIF_STATUS_WARNING                         (cpu_to_le32(0x0003))
+#define HIF_STATUS_UNKNOWN_REQUEST                 (cpu_to_le32(0x0004))
+#define HIF_STATUS_RX_FAIL_DECRYPT                 (cpu_to_le32(0x0010))
+#define HIF_STATUS_RX_FAIL_MIC                     (cpu_to_le32(0x0011))
+#define HIF_STATUS_RX_FAIL_NO_KEY                  (cpu_to_le32(0x0012))
+#define HIF_STATUS_TX_FAIL_RETRIES                 (cpu_to_le32(0x0013))
+#define HIF_STATUS_TX_FAIL_TIMEOUT                 (cpu_to_le32(0x0014))
+#define HIF_STATUS_TX_FAIL_REQUEUE                 (cpu_to_le32(0x0015))
+#define HIF_STATUS_REFUSED                         (cpu_to_le32(0x0016))
+#define HIF_STATUS_BUSY                            (cpu_to_le32(0x0017))
+#define HIF_STATUS_SLK_SET_KEY_SUCCESS             (cpu_to_le32(0x005A))
+#define HIF_STATUS_SLK_SET_KEY_ALREADY_BURNED      (cpu_to_le32(0x006B))
+#define HIF_STATUS_SLK_SET_KEY_DISALLOWED_MODE     (cpu_to_le32(0x007C))
+#define HIF_STATUS_SLK_SET_KEY_UNKNOWN_MODE        (cpu_to_le32(0x008D))
+#define HIF_STATUS_SLK_NEGO_SUCCESS                (cpu_to_le32(0x009E))
+#define HIF_STATUS_SLK_NEGO_FAILED                 (cpu_to_le32(0x00AF))
+#define HIF_STATUS_ROLLBACK_SUCCESS                (cpu_to_le32(0x1234))
+#define HIF_STATUS_ROLLBACK_FAIL                   (cpu_to_le32(0x1256))
 
 enum hif_api_rate_index {
 	API_RATE_INDEX_B_1MBPS     = 0,
diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index 36132909a6ae..88466063cc42 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -127,7 +127,7 @@ static int hif_keys_indication(struct wfx_dev *wdev,
 	u8 pubkey[API_NCP_PUB_KEY_SIZE];
 
 	// SL_PUB_KEY_EXCHANGE_STATUS_SUCCESS is used by legacy secure link
-	if (body->status && body->status != SL_PUB_KEY_EXCHANGE_STATUS_SUCCESS)
+	if (body->status && body->status != HIF_STATUS_SLK_NEGO_SUCCESS)
 		dev_warn(wdev->dev, "secure link negociation error\n");
 	memcpy(pubkey, body->ncp_pub_key, sizeof(pubkey));
 	memreverse(pubkey, sizeof(pubkey));
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 490a9de54faf..bb776ee6689c 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -511,7 +511,7 @@ int hif_sl_send_pub_keys(struct wfx_dev *wdev,
 	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
 	kfree(hif);
 	// Compatibility with legacy secure link
-	if (ret == SL_PUB_KEY_EXCHANGE_STATUS_SUCCESS)
+	if (ret == le32_to_cpu(HIF_STATUS_SLK_NEGO_SUCCESS))
 		ret = 0;
 	return ret;
 }
@@ -542,7 +542,7 @@ int hif_sl_set_mac_key(struct wfx_dev *wdev, const u8 *slk_key, int destination)
 	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
 	kfree(hif);
 	// Compatibility with legacy secure link
-	if (ret == SL_MAC_KEY_STATUS_SUCCESS)
+	if (ret == le32_to_cpu(HIF_STATUS_SLK_SET_KEY_SUCCESS))
 		ret = 0;
 	return ret;
 }
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 25d70ebe9933..d4e69c663f5a 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -229,7 +229,7 @@ int wfx_send_pds(struct wfx_dev *wdev, u8 *buf, size_t len)
 			buf[i] = '}';
 			ret = hif_configuration(wdev, buf + start,
 						i - start + 1);
-			if (ret == HIF_STATUS_FAILURE) {
+			if (ret > 0) {
 				dev_err(wdev->dev, "PDS bytes %d to %d: invalid data (unsupported options?)\n", start, i);
 				return -EINVAL;
 			}
diff --git a/drivers/staging/wfx/traces.h b/drivers/staging/wfx/traces.h
index 7298fb948f56..c10ac92b8eb3 100644
--- a/drivers/staging/wfx/traces.h
+++ b/drivers/staging/wfx/traces.h
@@ -414,7 +414,7 @@ TRACE_EVENT(tx_stats,
 			__entry->flags |= 0x10;
 		if (tx_cnf->status)
 			__entry->flags |= 0x20;
-		if (tx_cnf->status == HIF_REQUEUE)
+		if (tx_cnf->status == HIF_STATUS_TX_FAIL_REQUEUE)
 			__entry->flags |= 0x40;
 	),
 	TP_printk("packet ID: %08x, rate policy: %s %d|%d %d|%d %d|%d %d|%d -> %d attempt, Delays media/queue/total: %4dus/%4dus/%4dus",
-- 
2.26.2

Powered by blists - more mailing lists