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@xxxxxxxxx> Cc: Allen Pais <apais@xxxxxxxxxxxxxxxxxxx> Cc: Ivan Safonov <insafonov@xxxxxxxxx> Cc: Arnd Bergmann <arnd@xxxxxxxx> Cc: linux-staging@xxxxxxxxxxxxxxx Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> --- 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