All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] staging: wlan-ng: Avoid duplicate header in tx/rx frames
@ 2021-08-19 17:45 Kees Cook
  2021-08-19 17:45 ` [PATCH 1/2] staging: wlan-ng: Remove pointless a3/a4 union Kees Cook
  2021-08-19 17:45 ` [PATCH 2/2] staging: wlan-ng: Avoid duplicate header in tx/rx frames Kees Cook
  0 siblings, 2 replies; 3+ messages in thread
From: Kees Cook @ 2021-08-19 17:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Allen Pais, Romain Perier, Chen Lin, Ivan Safonov,
	Arnd Bergmann, David S. Miller, Abheek Dhawan, Colin Ian King,
	Ashish Kalra, linux-kernel, linux-staging, linux-hardening

Hi,

In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), memmove(), and memset(), avoid
intentionally writing across neighboring fields.

In this case, there is a lot of redundant struct member declarations,
and everything can be made smaller, more description, and drop a
field-spanning memcpy() call all at the same time. The result is no
warning from the future FORTIFY_SOURCE and identical machine code.

Thanks,

-Kees

Kees Cook (2):
  staging: wlan-ng: Remove pointless a3/a4 union
  staging: wlan-ng: Avoid duplicate header in tx/rx frames

 drivers/staging/wlan-ng/hfa384x.h      | 19 ++--------
 drivers/staging/wlan-ng/hfa384x_usb.c  | 13 ++++---
 drivers/staging/wlan-ng/p80211conv.c   | 48 +++++++++++++-------------
 drivers/staging/wlan-ng/p80211conv.h   |  2 +-
 drivers/staging/wlan-ng/p80211hdr.h    | 30 +++++-----------
 drivers/staging/wlan-ng/p80211mgmt.h   | 24 ++++++-------
 drivers/staging/wlan-ng/p80211netdev.c | 12 +++----
 drivers/staging/wlan-ng/p80211netdev.h |  2 +-
 drivers/staging/wlan-ng/prism2sta.c    |  6 ++--
 9 files changed, 64 insertions(+), 92 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/2] staging: wlan-ng: Remove pointless a3/a4 union
  2021-08-19 17:45 [PATCH 0/2] staging: wlan-ng: Avoid duplicate header in tx/rx frames Kees Cook
@ 2021-08-19 17:45 ` Kees Cook
  2021-08-19 17:45 ` [PATCH 2/2] staging: wlan-ng: Avoid duplicate header in tx/rx frames Kees Cook
  1 sibling, 0 replies; 3+ messages in thread
From: Kees Cook @ 2021-08-19 17:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Allen Pais, Romain Perier, Chen Lin, Ivan Safonov,
	Arnd Bergmann, linux-staging, David S. Miller, Abheek Dhawan,
	Colin Ian King, Ashish Kalra, linux-kernel, linux-hardening

There is no need for the a3/a4 union. The two structs are identical
except for the addition of a4. Excepting one place, the structs are
only ever used in the union, and the union is always allocated at full
size. The one instance of the a3-specific struct can be replaced with
the full version, as no sizing information is used. Replace the union
with the a4 version of the struct. "diffoscope" reports there are no
object code differences after this change.

Cc: Allen Pais <apais@linux.microsoft.com>
Cc: Romain Perier <romain.perier@gmail.com>
Cc: Chen Lin <chen.lin5@zte.com.cn>
Cc: Ivan Safonov <insafonov@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-staging@lists.linux.dev
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/staging/wlan-ng/hfa384x.h      |  2 +-
 drivers/staging/wlan-ng/hfa384x_usb.c  |  4 +--
 drivers/staging/wlan-ng/p80211conv.c   | 48 +++++++++++++-------------
 drivers/staging/wlan-ng/p80211conv.h   |  2 +-
 drivers/staging/wlan-ng/p80211hdr.h    | 16 +--------
 drivers/staging/wlan-ng/p80211mgmt.h   | 24 ++++++-------
 drivers/staging/wlan-ng/p80211netdev.c |  6 ++--
 drivers/staging/wlan-ng/p80211netdev.h |  2 +-
 drivers/staging/wlan-ng/prism2sta.c    |  6 ++--
 9 files changed, 48 insertions(+), 62 deletions(-)

diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h
index 88e894dd3568..f1bc1f2816af 100644
--- a/drivers/staging/wlan-ng/hfa384x.h
+++ b/drivers/staging/wlan-ng/hfa384x.h
@@ -1423,7 +1423,7 @@ int hfa384x_drvr_start(struct hfa384x *hw);
 int hfa384x_drvr_stop(struct hfa384x *hw);
 int
 hfa384x_drvr_txframe(struct hfa384x *hw, struct sk_buff *skb,
-		     union p80211_hdr *p80211_hdr,
+		     struct p80211_hdr *p80211_hdr,
 		     struct p80211_metawep *p80211_wep);
 void hfa384x_tx_timeout(struct wlandevice *wlandev);
 
diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c b/drivers/staging/wlan-ng/hfa384x_usb.c
index f2a0e16b0318..0bf71f395b37 100644
--- a/drivers/staging/wlan-ng/hfa384x_usb.c
+++ b/drivers/staging/wlan-ng/hfa384x_usb.c
@@ -2472,7 +2472,7 @@ int hfa384x_drvr_stop(struct hfa384x *hw)
  *----------------------------------------------------------------
  */
 int hfa384x_drvr_txframe(struct hfa384x *hw, struct sk_buff *skb,
-			 union p80211_hdr *p80211_hdr,
+			 struct p80211_hdr *p80211_hdr,
 			 struct p80211_metawep *p80211_wep)
 {
 	int usbpktlen = sizeof(struct hfa384x_tx_frame);
@@ -2517,7 +2517,7 @@ int hfa384x_drvr_txframe(struct hfa384x *hw, struct sk_buff *skb,
 
 	/* copy the header over to the txdesc */
 	memcpy(&hw->txbuff.txfrm.desc.frame_control, p80211_hdr,
-	       sizeof(union p80211_hdr));
+	       sizeof(struct p80211_hdr));
 
 	/* if we're using host WEP, increase size by IV+ICV */
 	if (p80211_wep->data) {
diff --git a/drivers/staging/wlan-ng/p80211conv.c b/drivers/staging/wlan-ng/p80211conv.c
index 0ff5fda81b05..0b3ba03c1f1f 100644
--- a/drivers/staging/wlan-ng/p80211conv.c
+++ b/drivers/staging/wlan-ng/p80211conv.c
@@ -106,7 +106,7 @@ static const u8 oui_8021h[] = { 0x00, 0x00, 0xf8 };
  *----------------------------------------------------------------
  */
 int skb_ether_to_p80211(struct wlandevice *wlandev, u32 ethconv,
-			struct sk_buff *skb, union p80211_hdr *p80211_hdr,
+			struct sk_buff *skb, struct p80211_hdr *p80211_hdr,
 			struct p80211_metawep *p80211_wep)
 {
 	__le16 fc;
@@ -175,21 +175,21 @@ int skb_ether_to_p80211(struct wlandevice *wlandev, u32 ethconv,
 
 	switch (wlandev->macmode) {
 	case WLAN_MACMODE_IBSS_STA:
-		memcpy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a3, wlandev->bssid, ETH_ALEN);
+		memcpy(p80211_hdr->a1, &e_hdr.daddr, ETH_ALEN);
+		memcpy(p80211_hdr->a2, wlandev->netdev->dev_addr, ETH_ALEN);
+		memcpy(p80211_hdr->a3, wlandev->bssid, ETH_ALEN);
 		break;
 	case WLAN_MACMODE_ESS_STA:
 		fc |= cpu_to_le16(WLAN_SET_FC_TODS(1));
-		memcpy(p80211_hdr->a3.a1, wlandev->bssid, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a2, wlandev->netdev->dev_addr, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a3, &e_hdr.daddr, ETH_ALEN);
+		memcpy(p80211_hdr->a1, wlandev->bssid, ETH_ALEN);
+		memcpy(p80211_hdr->a2, wlandev->netdev->dev_addr, ETH_ALEN);
+		memcpy(p80211_hdr->a3, &e_hdr.daddr, ETH_ALEN);
 		break;
 	case WLAN_MACMODE_ESS_AP:
 		fc |= cpu_to_le16(WLAN_SET_FC_FROMDS(1));
-		memcpy(p80211_hdr->a3.a1, &e_hdr.daddr, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a2, wlandev->bssid, ETH_ALEN);
-		memcpy(p80211_hdr->a3.a3, &e_hdr.saddr, ETH_ALEN);
+		memcpy(p80211_hdr->a1, &e_hdr.daddr, ETH_ALEN);
+		memcpy(p80211_hdr->a2, wlandev->bssid, ETH_ALEN);
+		memcpy(p80211_hdr->a3, &e_hdr.saddr, ETH_ALEN);
 		break;
 	default:
 		netdev_err(wlandev->netdev,
@@ -222,9 +222,9 @@ int skb_ether_to_p80211(struct wlandevice *wlandev, u32 ethconv,
 
 	/*      skb->nh.raw = skb->data; */
 
-	p80211_hdr->a3.fc = fc;
-	p80211_hdr->a3.dur = 0;
-	p80211_hdr->a3.seq = 0;
+	p80211_hdr->fc = fc;
+	p80211_hdr->dur = 0;
+	p80211_hdr->seq = 0;
 
 	return 0;
 }
@@ -281,7 +281,7 @@ int skb_p80211_to_ether(struct wlandevice *wlandev, u32 ethconv,
 	unsigned int payload_offset;
 	u8 daddr[ETH_ALEN];
 	u8 saddr[ETH_ALEN];
-	union p80211_hdr *w_hdr;
+	struct p80211_hdr *w_hdr;
 	struct wlan_ethhdr *e_hdr;
 	struct wlan_llc *e_llc;
 	struct wlan_snap *e_snap;
@@ -291,21 +291,21 @@ int skb_p80211_to_ether(struct wlandevice *wlandev, u32 ethconv,
 	payload_length = skb->len - WLAN_HDR_A3_LEN - WLAN_CRC_LEN;
 	payload_offset = WLAN_HDR_A3_LEN;
 
-	w_hdr = (union p80211_hdr *)skb->data;
+	w_hdr = (struct p80211_hdr *)skb->data;
 
 	/* setup some vars for convenience */
-	fc = le16_to_cpu(w_hdr->a3.fc);
+	fc = le16_to_cpu(w_hdr->fc);
 	if ((WLAN_GET_FC_TODS(fc) == 0) && (WLAN_GET_FC_FROMDS(fc) == 0)) {
-		ether_addr_copy(daddr, w_hdr->a3.a1);
-		ether_addr_copy(saddr, w_hdr->a3.a2);
+		ether_addr_copy(daddr, w_hdr->a1);
+		ether_addr_copy(saddr, w_hdr->a2);
 	} else if ((WLAN_GET_FC_TODS(fc) == 0) &&
 		   (WLAN_GET_FC_FROMDS(fc) == 1)) {
-		ether_addr_copy(daddr, w_hdr->a3.a1);
-		ether_addr_copy(saddr, w_hdr->a3.a3);
+		ether_addr_copy(daddr, w_hdr->a1);
+		ether_addr_copy(saddr, w_hdr->a3);
 	} else if ((WLAN_GET_FC_TODS(fc) == 1) &&
 		   (WLAN_GET_FC_FROMDS(fc) == 0)) {
-		ether_addr_copy(daddr, w_hdr->a3.a3);
-		ether_addr_copy(saddr, w_hdr->a3.a2);
+		ether_addr_copy(daddr, w_hdr->a3);
+		ether_addr_copy(saddr, w_hdr->a2);
 	} else {
 		payload_offset = WLAN_HDR_A4_LEN;
 		if (payload_length < WLAN_HDR_A4_LEN - WLAN_HDR_A3_LEN) {
@@ -313,8 +313,8 @@ int skb_p80211_to_ether(struct wlandevice *wlandev, u32 ethconv,
 			return 1;
 		}
 		payload_length -= (WLAN_HDR_A4_LEN - WLAN_HDR_A3_LEN);
-		ether_addr_copy(daddr, w_hdr->a4.a3);
-		ether_addr_copy(saddr, w_hdr->a4.a4);
+		ether_addr_copy(daddr, w_hdr->a3);
+		ether_addr_copy(saddr, w_hdr->a4);
 	}
 
 	/* perform de-wep if necessary.. */
diff --git a/drivers/staging/wlan-ng/p80211conv.h b/drivers/staging/wlan-ng/p80211conv.h
index 15fd635d9770..63c423507fe8 100644
--- a/drivers/staging/wlan-ng/p80211conv.h
+++ b/drivers/staging/wlan-ng/p80211conv.h
@@ -154,7 +154,7 @@ struct wlandevice;
 int skb_p80211_to_ether(struct wlandevice *wlandev, u32 ethconv,
 			struct sk_buff *skb);
 int skb_ether_to_p80211(struct wlandevice *wlandev, u32 ethconv,
-			struct sk_buff *skb, union p80211_hdr *p80211_hdr,
+			struct sk_buff *skb, struct p80211_hdr *p80211_hdr,
 			struct p80211_metawep *p80211_wep);
 
 int p80211_stt_findproto(u16 proto);
diff --git a/drivers/staging/wlan-ng/p80211hdr.h b/drivers/staging/wlan-ng/p80211hdr.h
index 6564810fd026..93dd8ff1940c 100644
--- a/drivers/staging/wlan-ng/p80211hdr.h
+++ b/drivers/staging/wlan-ng/p80211hdr.h
@@ -148,16 +148,7 @@
 
 /* Generic 802.11 Header types */
 
-struct p80211_hdr_a3 {
-	__le16 fc;
-	u16 dur;
-	u8 a1[ETH_ALEN];
-	u8 a2[ETH_ALEN];
-	u8 a3[ETH_ALEN];
-	u16 seq;
-} __packed;
-
-struct p80211_hdr_a4 {
+struct p80211_hdr {
 	u16 fc;
 	u16 dur;
 	u8 a1[ETH_ALEN];
@@ -167,11 +158,6 @@ struct p80211_hdr_a4 {
 	u8 a4[ETH_ALEN];
 } __packed;
 
-union p80211_hdr {
-	struct p80211_hdr_a3 a3;
-	struct p80211_hdr_a4 a4;
-} __packed;
-
 /* Frame and header length macros */
 
 static inline u16 wlan_ctl_framelen(u16 fstype)
diff --git a/drivers/staging/wlan-ng/p80211mgmt.h b/drivers/staging/wlan-ng/p80211mgmt.h
index c045c08e1991..1457a6def5a2 100644
--- a/drivers/staging/wlan-ng/p80211mgmt.h
+++ b/drivers/staging/wlan-ng/p80211mgmt.h
@@ -299,7 +299,7 @@ struct wlan_fr_mgmt {
 	u16 type;
 	u16 len;		/* DOES NOT include CRC !!!! */
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 	/*-- fixed fields -----------*/
@@ -311,7 +311,7 @@ struct wlan_fr_beacon {
 	u16 type;
 	u16 len;
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 	/*-- fixed fields -----------*/
@@ -334,7 +334,7 @@ struct wlan_fr_ibssatim {
 	u16 type;
 	u16 len;
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 
@@ -350,7 +350,7 @@ struct wlan_fr_disassoc {
 	u16 type;
 	u16 len;
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 	/*-- fixed fields -----------*/
@@ -365,7 +365,7 @@ struct wlan_fr_assocreq {
 	u16 type;
 	u16 len;
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 	/*-- fixed fields -----------*/
@@ -382,7 +382,7 @@ struct wlan_fr_assocresp {
 	u16 type;
 	u16 len;
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 	/*-- fixed fields -----------*/
@@ -399,7 +399,7 @@ struct wlan_fr_reassocreq {
 	u16 type;
 	u16 len;
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 	/*-- fixed fields -----------*/
@@ -417,7 +417,7 @@ struct wlan_fr_reassocresp {
 	u16 type;
 	u16 len;
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 	/*-- fixed fields -----------*/
@@ -434,7 +434,7 @@ struct wlan_fr_probereq {
 	u16 type;
 	u16 len;
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 	/*-- fixed fields -----------*/
@@ -449,7 +449,7 @@ struct wlan_fr_proberesp {
 	u16 type;
 	u16 len;
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 	/*-- fixed fields -----------*/
@@ -470,7 +470,7 @@ struct wlan_fr_authen {
 	u16 type;
 	u16 len;
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 	/*-- fixed fields -----------*/
@@ -487,7 +487,7 @@ struct wlan_fr_deauthen {
 	u16 type;
 	u16 len;
 	u8 *buf;
-	union p80211_hdr *hdr;
+	struct p80211_hdr *hdr;
 	/* used for target specific data, skb in Linux */
 	void *priv;
 	/*-- fixed fields -----------*/
diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
index 1c62130a5eee..53cbac890614 100644
--- a/drivers/staging/wlan-ng/p80211netdev.c
+++ b/drivers/staging/wlan-ng/p80211netdev.c
@@ -235,9 +235,9 @@ void p80211netdev_rx(struct wlandevice *wlandev, struct sk_buff *skb)
 static int p80211_convert_to_ether(struct wlandevice *wlandev,
 				   struct sk_buff *skb)
 {
-	struct p80211_hdr_a3 *hdr;
+	struct p80211_hdr *hdr;
 
-	hdr = (struct p80211_hdr_a3 *)skb->data;
+	hdr = (struct p80211_hdr *)skb->data;
 	if (p80211_rx_typedrop(wlandev, le16_to_cpu(hdr->fc)))
 		return CONV_TO_ETHER_SKIPPED;
 
@@ -327,7 +327,7 @@ static netdev_tx_t p80211knetdev_hard_start_xmit(struct sk_buff *skb,
 	int result = 0;
 	int txresult;
 	struct wlandevice *wlandev = netdev->ml_priv;
-	union p80211_hdr p80211_hdr;
+	struct p80211_hdr p80211_hdr;
 	struct p80211_metawep p80211_wep;
 
 	p80211_wep.data = NULL;
diff --git a/drivers/staging/wlan-ng/p80211netdev.h b/drivers/staging/wlan-ng/p80211netdev.h
index d48466d943b4..25e5116b1590 100644
--- a/drivers/staging/wlan-ng/p80211netdev.h
+++ b/drivers/staging/wlan-ng/p80211netdev.h
@@ -180,7 +180,7 @@ struct wlandevice {
 	int (*close)(struct wlandevice *wlandev);
 	void (*reset)(struct wlandevice *wlandev);
 	int (*txframe)(struct wlandevice *wlandev, struct sk_buff *skb,
-		       union p80211_hdr *p80211_hdr,
+		       struct p80211_hdr *p80211_hdr,
 		       struct p80211_metawep *p80211_wep);
 	int (*mlmerequest)(struct wlandevice *wlandev, struct p80211msg *msg);
 	int (*set_multicast_list)(struct wlandevice *wlandev,
diff --git a/drivers/staging/wlan-ng/prism2sta.c b/drivers/staging/wlan-ng/prism2sta.c
index e6dcb687e7a1..1f9ba26f1f36 100644
--- a/drivers/staging/wlan-ng/prism2sta.c
+++ b/drivers/staging/wlan-ng/prism2sta.c
@@ -103,7 +103,7 @@ static int prism2sta_open(struct wlandevice *wlandev);
 static int prism2sta_close(struct wlandevice *wlandev);
 static void prism2sta_reset(struct wlandevice *wlandev);
 static int prism2sta_txframe(struct wlandevice *wlandev, struct sk_buff *skb,
-			     union p80211_hdr *p80211_hdr,
+			     struct p80211_hdr *p80211_hdr,
 			     struct p80211_metawep *p80211_wep);
 static int prism2sta_mlmerequest(struct wlandevice *wlandev,
 				 struct p80211msg *msg);
@@ -242,7 +242,7 @@ static void prism2sta_reset(struct wlandevice *wlandev)
  *	process thread
  */
 static int prism2sta_txframe(struct wlandevice *wlandev, struct sk_buff *skb,
-			     union p80211_hdr *p80211_hdr,
+			     struct p80211_hdr *p80211_hdr,
 			     struct p80211_metawep *p80211_wep)
 {
 	struct hfa384x *hw = wlandev->priv;
@@ -250,7 +250,7 @@ static int prism2sta_txframe(struct wlandevice *wlandev, struct sk_buff *skb,
 	/* If necessary, set the 802.11 WEP bit */
 	if ((wlandev->hostwep & (HOSTWEP_PRIVACYINVOKED | HOSTWEP_ENCRYPT)) ==
 	    HOSTWEP_PRIVACYINVOKED) {
-		p80211_hdr->a3.fc |= cpu_to_le16(WLAN_SET_FC_ISWEP(1));
+		p80211_hdr->fc |= cpu_to_le16(WLAN_SET_FC_ISWEP(1));
 	}
 
 	return hfa384x_drvr_txframe(hw, skb, p80211_hdr, p80211_wep);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] staging: wlan-ng: Avoid duplicate header in tx/rx frames
  2021-08-19 17:45 [PATCH 0/2] staging: wlan-ng: Avoid duplicate header in tx/rx frames Kees Cook
  2021-08-19 17:45 ` [PATCH 1/2] staging: wlan-ng: Remove pointless a3/a4 union Kees Cook
@ 2021-08-19 17:45 ` Kees Cook
  1 sibling, 0 replies; 3+ messages in thread
From: Kees Cook @ 2021-08-19 17:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Romain Perier, Allen Pais, Ivan Safonov,
	Arnd Bergmann, linux-staging, Chen Lin, David S. Miller,
	Abheek Dhawan, Colin Ian King, Ashish Kalra, linux-kernel,
	linux-hardening

Instead of open-coding the same header details in the tx/rx frames,
directly include the actual struct. Rename associated variables to the
more verbose of the two versions. This also has the benefit of being
able to replace a field-spanning memcpy() with a direct assignment,
helping clear the way for coming FORTIFY_SOURCE improvements.

"diffoscope" reports no object code differences after this change,
excepting the selection of different registers when switching from
memcpy() to direct assignment:

 --- drivers/staging/wlan-ng/prism2usb.o.before
 +++ drivers/staging/wlan-ng/prism2usb.o.after
├── objdump --line-numbers --disassemble --demangle --reloc --no-show-raw-insn --section=.text {}
│ @@ -4887,24 +4887,24 @@
│       sub    %rdi,%rcx
│       add    $0x3c,%ecx
│       shr    $0x3,%ecx
│       rep stos %rax,%es:(%rdi)
│       mov    $0x8,%eax
│       movl   $0x123,0x23e(%rbx)
│       mov    %ax,0x244(%rbx)
│ -     mov    (%rdx),%rcx
│ -     mov    %rcx,0x246(%rbx)
│ -     mov    0x8(%rdx),%rcx
│ -     mov    %rcx,0x24e(%rbx)
│ -     mov    0x10(%rdx),%rcx
│ -     mov    %rcx,0x256(%rbx)
│ -     mov    0x18(%rdx),%ecx
│ -     mov    %ecx,0x25e(%rbx)
│ -     movzwl 0x1c(%rdx),%edx
│ -     mov    %dx,0x262(%rbx)
│ +     mov    (%rdx),%rax
│ +     mov    %rax,0x246(%rbx)
│ +     mov    0x8(%rdx),%rax
│ +     mov    %rax,0x24e(%rbx)
│ +     mov    0x10(%rdx),%rax
│ +     mov    %rax,0x256(%rbx)
│ +     mov    0x18(%rdx),%eax
│ +     mov    %eax,0x25e(%rbx)
│ +     movzwl 0x1c(%rdx),%eax
│ +     mov    %ax,0x262(%rbx)
│       cmpq   $0x0,0x0(%rbp)
│       movzwl 0x70(%rsi),%eax
│       je     477a <hfa384x_drvr_txframe+0xba>
│       add    $0x8,%eax
│       mov    $0x44,%r12d
│       mov    %ax,0x264(%rbx)
│       mov    0x70(%r13),%edx

Cc: Romain Perier <romain.perier@gmail.com>
Cc: Allen Pais <apais@linux.microsoft.com>
Cc: Ivan Safonov <insafonov@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-staging@lists.linux.dev
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/staging/wlan-ng/hfa384x.h      | 17 ++---------
 drivers/staging/wlan-ng/hfa384x_usb.c  | 11 +++----
 drivers/staging/wlan-ng/p80211conv.c   | 42 +++++++++++++-------------
 drivers/staging/wlan-ng/p80211hdr.h    | 14 ++++-----
 drivers/staging/wlan-ng/p80211netdev.c |  6 ++--
 drivers/staging/wlan-ng/prism2sta.c    |  2 +-
 6 files changed, 39 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h
index f1bc1f2816af..75ed8bc4bbc1 100644
--- a/drivers/staging/wlan-ng/hfa384x.h
+++ b/drivers/staging/wlan-ng/hfa384x.h
@@ -475,14 +475,7 @@ struct hfa384x_tx_frame {
 	u16 tx_control;
 
 	/*-- 802.11 Header Information --*/
-
-	u16 frame_control;
-	u16 duration_id;
-	u8 address1[6];
-	u8 address2[6];
-	u8 address3[6];
-	u16 sequence_control;
-	u8 address4[6];
+	struct p80211_hdr hdr;
 	__le16 data_len;		/* little endian format */
 
 	/*-- 802.3 Header Information --*/
@@ -541,13 +534,7 @@ struct hfa384x_rx_frame {
 	u16 reserved2;
 
 	/*-- 802.11 Header Information (802.11 byte order) --*/
-	__le16 frame_control;
-	u16 duration_id;
-	u8 address1[6];
-	u8 address2[6];
-	u8 address3[6];
-	u16 sequence_control;
-	u8 address4[6];
+	struct p80211_hdr hdr;
 	__le16 data_len;		/* hfa384x (little endian) format */
 
 	/*-- 802.3 Header Information --*/
diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c b/drivers/staging/wlan-ng/hfa384x_usb.c
index 0bf71f395b37..8c8524679ba3 100644
--- a/drivers/staging/wlan-ng/hfa384x_usb.c
+++ b/drivers/staging/wlan-ng/hfa384x_usb.c
@@ -2516,8 +2516,7 @@ int hfa384x_drvr_txframe(struct hfa384x *hw, struct sk_buff *skb,
 	cpu_to_le16s(&hw->txbuff.txfrm.desc.tx_control);
 
 	/* copy the header over to the txdesc */
-	memcpy(&hw->txbuff.txfrm.desc.frame_control, p80211_hdr,
-	       sizeof(struct p80211_hdr));
+	hw->txbuff.txfrm.desc.hdr = *p80211_hdr;
 
 	/* if we're using host WEP, increase size by IV+ICV */
 	if (p80211_wep->data) {
@@ -3258,7 +3257,7 @@ static void hfa384x_usbin_rx(struct wlandevice *wlandev, struct sk_buff *skb)
 
 	switch (status) {
 	case 0:
-		fc = le16_to_cpu(usbin->rxfrm.desc.frame_control);
+		fc = le16_to_cpu(usbin->rxfrm.desc.hdr.frame_control);
 
 		/* If exclude and we receive an unencrypted, drop it */
 		if ((wlandev->hostwep & HOSTWEP_EXCLUDEUNENCRYPTED) &&
@@ -3278,7 +3277,7 @@ static void hfa384x_usbin_rx(struct wlandevice *wlandev, struct sk_buff *skb)
 		 * with an "overlapping" copy
 		 */
 		memmove(skb_push(skb, hdrlen),
-			&usbin->rxfrm.desc.frame_control, hdrlen);
+			&usbin->rxfrm.desc.hdr, hdrlen);
 
 		skb->dev = wlandev->netdev;
 
@@ -3356,7 +3355,7 @@ static void hfa384x_int_rxmonitor(struct wlandevice *wlandev,
 
 	/* Remember the status, time, and data_len fields are in host order */
 	/* Figure out how big the frame is */
-	fc = le16_to_cpu(rxdesc->frame_control);
+	fc = le16_to_cpu(rxdesc->hdr.frame_control);
 	hdrlen = p80211_headerlen(fc);
 	datalen = le16_to_cpu(rxdesc->data_len);
 
@@ -3404,7 +3403,7 @@ static void hfa384x_int_rxmonitor(struct wlandevice *wlandev,
 	/* Copy the 802.11 header to the skb
 	 * (ctl frames may be less than a full header)
 	 */
-	skb_put_data(skb, &rxdesc->frame_control, hdrlen);
+	skb_put_data(skb, &rxdesc->hdr.frame_control, hdrlen);
 
 	/* If any, copy the data from the card to the skb */
 	if (datalen > 0) {
diff --git a/drivers/staging/wlan-ng/p80211conv.c b/drivers/staging/wlan-ng/p80211conv.c
index 0b3ba03c1f1f..59b25ca50d15 100644
--- a/drivers/staging/wlan-ng/p80211conv.c
+++ b/drivers/staging/wlan-ng/p80211conv.c
@@ -175,21 +175,21 @@ int skb_ether_to_p80211(struct wlandevice *wlandev, u32 ethconv,
 
 	switch (wlandev->macmode) {
 	case WLAN_MACMODE_IBSS_STA:
-		memcpy(p80211_hdr->a1, &e_hdr.daddr, ETH_ALEN);
-		memcpy(p80211_hdr->a2, wlandev->netdev->dev_addr, ETH_ALEN);
-		memcpy(p80211_hdr->a3, wlandev->bssid, ETH_ALEN);
+		memcpy(p80211_hdr->address1, &e_hdr.daddr, ETH_ALEN);
+		memcpy(p80211_hdr->address2, wlandev->netdev->dev_addr, ETH_ALEN);
+		memcpy(p80211_hdr->address3, wlandev->bssid, ETH_ALEN);
 		break;
 	case WLAN_MACMODE_ESS_STA:
 		fc |= cpu_to_le16(WLAN_SET_FC_TODS(1));
-		memcpy(p80211_hdr->a1, wlandev->bssid, ETH_ALEN);
-		memcpy(p80211_hdr->a2, wlandev->netdev->dev_addr, ETH_ALEN);
-		memcpy(p80211_hdr->a3, &e_hdr.daddr, ETH_ALEN);
+		memcpy(p80211_hdr->address1, wlandev->bssid, ETH_ALEN);
+		memcpy(p80211_hdr->address2, wlandev->netdev->dev_addr, ETH_ALEN);
+		memcpy(p80211_hdr->address3, &e_hdr.daddr, ETH_ALEN);
 		break;
 	case WLAN_MACMODE_ESS_AP:
 		fc |= cpu_to_le16(WLAN_SET_FC_FROMDS(1));
-		memcpy(p80211_hdr->a1, &e_hdr.daddr, ETH_ALEN);
-		memcpy(p80211_hdr->a2, wlandev->bssid, ETH_ALEN);
-		memcpy(p80211_hdr->a3, &e_hdr.saddr, ETH_ALEN);
+		memcpy(p80211_hdr->address1, &e_hdr.daddr, ETH_ALEN);
+		memcpy(p80211_hdr->address2, wlandev->bssid, ETH_ALEN);
+		memcpy(p80211_hdr->address3, &e_hdr.saddr, ETH_ALEN);
 		break;
 	default:
 		netdev_err(wlandev->netdev,
@@ -222,9 +222,9 @@ int skb_ether_to_p80211(struct wlandevice *wlandev, u32 ethconv,
 
 	/*      skb->nh.raw = skb->data; */
 
-	p80211_hdr->fc = fc;
-	p80211_hdr->dur = 0;
-	p80211_hdr->seq = 0;
+	p80211_hdr->frame_control = fc;
+	p80211_hdr->duration_id = 0;
+	p80211_hdr->sequence_control = 0;
 
 	return 0;
 }
@@ -294,18 +294,18 @@ int skb_p80211_to_ether(struct wlandevice *wlandev, u32 ethconv,
 	w_hdr = (struct p80211_hdr *)skb->data;
 
 	/* setup some vars for convenience */
-	fc = le16_to_cpu(w_hdr->fc);
+	fc = le16_to_cpu(w_hdr->frame_control);
 	if ((WLAN_GET_FC_TODS(fc) == 0) && (WLAN_GET_FC_FROMDS(fc) == 0)) {
-		ether_addr_copy(daddr, w_hdr->a1);
-		ether_addr_copy(saddr, w_hdr->a2);
+		ether_addr_copy(daddr, w_hdr->address1);
+		ether_addr_copy(saddr, w_hdr->address2);
 	} else if ((WLAN_GET_FC_TODS(fc) == 0) &&
 		   (WLAN_GET_FC_FROMDS(fc) == 1)) {
-		ether_addr_copy(daddr, w_hdr->a1);
-		ether_addr_copy(saddr, w_hdr->a3);
+		ether_addr_copy(daddr, w_hdr->address1);
+		ether_addr_copy(saddr, w_hdr->address3);
 	} else if ((WLAN_GET_FC_TODS(fc) == 1) &&
 		   (WLAN_GET_FC_FROMDS(fc) == 0)) {
-		ether_addr_copy(daddr, w_hdr->a3);
-		ether_addr_copy(saddr, w_hdr->a2);
+		ether_addr_copy(daddr, w_hdr->address3);
+		ether_addr_copy(saddr, w_hdr->address2);
 	} else {
 		payload_offset = WLAN_HDR_A4_LEN;
 		if (payload_length < WLAN_HDR_A4_LEN - WLAN_HDR_A3_LEN) {
@@ -313,8 +313,8 @@ int skb_p80211_to_ether(struct wlandevice *wlandev, u32 ethconv,
 			return 1;
 		}
 		payload_length -= (WLAN_HDR_A4_LEN - WLAN_HDR_A3_LEN);
-		ether_addr_copy(daddr, w_hdr->a3);
-		ether_addr_copy(saddr, w_hdr->a4);
+		ether_addr_copy(daddr, w_hdr->address3);
+		ether_addr_copy(saddr, w_hdr->address4);
 	}
 
 	/* perform de-wep if necessary.. */
diff --git a/drivers/staging/wlan-ng/p80211hdr.h b/drivers/staging/wlan-ng/p80211hdr.h
index 93dd8ff1940c..dd1fb99bf340 100644
--- a/drivers/staging/wlan-ng/p80211hdr.h
+++ b/drivers/staging/wlan-ng/p80211hdr.h
@@ -149,13 +149,13 @@
 /* Generic 802.11 Header types */
 
 struct p80211_hdr {
-	u16 fc;
-	u16 dur;
-	u8 a1[ETH_ALEN];
-	u8 a2[ETH_ALEN];
-	u8 a3[ETH_ALEN];
-	u16 seq;
-	u8 a4[ETH_ALEN];
+	u16	frame_control;
+	u16	duration_id;
+	u8	address1[ETH_ALEN];
+	u8	address2[ETH_ALEN];
+	u8	address3[ETH_ALEN];
+	u16	sequence_control;
+	u8	address4[ETH_ALEN];
 } __packed;
 
 /* Frame and header length macros */
diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
index 53cbac890614..2a3f9385ab3f 100644
--- a/drivers/staging/wlan-ng/p80211netdev.c
+++ b/drivers/staging/wlan-ng/p80211netdev.c
@@ -238,7 +238,7 @@ static int p80211_convert_to_ether(struct wlandevice *wlandev,
 	struct p80211_hdr *hdr;
 
 	hdr = (struct p80211_hdr *)skb->data;
-	if (p80211_rx_typedrop(wlandev, le16_to_cpu(hdr->fc)))
+	if (p80211_rx_typedrop(wlandev, le16_to_cpu(hdr->frame_control)))
 		return CONV_TO_ETHER_SKIPPED;
 
 	/* perform mcast filtering: allow my local address through but reject
@@ -246,8 +246,8 @@ static int p80211_convert_to_ether(struct wlandevice *wlandev,
 	 */
 	if (wlandev->netdev->flags & IFF_ALLMULTI) {
 		if (!ether_addr_equal_unaligned(wlandev->netdev->dev_addr,
-						hdr->a1)) {
-			if (!is_multicast_ether_addr(hdr->a1))
+						hdr->address1)) {
+			if (!is_multicast_ether_addr(hdr->address1))
 				return CONV_TO_ETHER_SKIPPED;
 		}
 	}
diff --git a/drivers/staging/wlan-ng/prism2sta.c b/drivers/staging/wlan-ng/prism2sta.c
index 1f9ba26f1f36..f67b7405156a 100644
--- a/drivers/staging/wlan-ng/prism2sta.c
+++ b/drivers/staging/wlan-ng/prism2sta.c
@@ -250,7 +250,7 @@ static int prism2sta_txframe(struct wlandevice *wlandev, struct sk_buff *skb,
 	/* If necessary, set the 802.11 WEP bit */
 	if ((wlandev->hostwep & (HOSTWEP_PRIVACYINVOKED | HOSTWEP_ENCRYPT)) ==
 	    HOSTWEP_PRIVACYINVOKED) {
-		p80211_hdr->fc |= cpu_to_le16(WLAN_SET_FC_ISWEP(1));
+		p80211_hdr->frame_control |= cpu_to_le16(WLAN_SET_FC_ISWEP(1));
 	}
 
 	return hfa384x_drvr_txframe(hw, skb, p80211_hdr, p80211_wep);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-19 17:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-19 17:45 [PATCH 0/2] staging: wlan-ng: Avoid duplicate header in tx/rx frames Kees Cook
2021-08-19 17:45 ` [PATCH 1/2] staging: wlan-ng: Remove pointless a3/a4 union Kees Cook
2021-08-19 17:45 ` [PATCH 2/2] staging: wlan-ng: Avoid duplicate header in tx/rx frames Kees Cook

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.