On Fri, Aug 13, 2021 at 10:04:09AM +0200, Johannes Berg wrote:
On Tue, 2021-07-27 at 13:58 -0700, Kees Cook wrote:
+++ b/include/linux/ieee80211.h @@ -297,9 +297,11 @@ static inline u16 ieee80211_sn_sub(u16 sn1, u16 sn2) struct ieee80211_hdr { __le16 frame_control; __le16 duration_id;
- u8 addr1[ETH_ALEN];
- u8 addr2[ETH_ALEN];
- u8 addr3[ETH_ALEN];
- struct_group(addrs,
u8 addr1[ETH_ALEN];u8 addr2[ETH_ALEN];u8 addr3[ETH_ALEN];- );
__le16 seq_ctrl; u8 addr4[ETH_ALEN]; } __packed __aligned(2);
This file isn't really just lib80211, it's also used by everyone else for 802.11, but I guess that's OK - after all, this doesn't really result in any changes here.
+++ b/net/wireless/lib80211_crypt_ccmp.c @@ -136,7 +136,8 @@ static int ccmp_init_iv_and_aad(const struct ieee80211_hdr *hdr, pos = (u8 *) hdr; aad[0] = pos[0] & 0x8f; aad[1] = pos[1] & 0xc7;
- memcpy(aad + 2, hdr->addr1, 3 * ETH_ALEN);
- BUILD_BUG_ON(sizeof(hdr->addrs) != 3 * ETH_ALEN);
- memcpy(aad + 2, &hdr->addrs, ETH_ALEN);
However, how is it you don't need the same change in net/mac80211/wpa.c?
We have three similar instances:
/* AAD (extra authenticate-only data) / masked 802.11 header * FC | A1 | A2 | A3 | SC | [A4] | [QC] */ put_unaligned_be16(len_a, &aad[0]); put_unaligned(mask_fc, (__le16 *)&aad[2]); memcpy(&aad[4], &hdr->addr1, 3 * ETH_ALEN);and
memcpy(&aad[4], &hdr->addr1, 3 * ETH_ALEN);and
memcpy(aad + 2, &hdr->addr1, 3 * ETH_ALEN);so those should also be changed, it seems?
Ah! Yes, thanks for pointing this out. During earlier development I split the "cross-field write" changes from the "cross-field read" changes, and it looks like I missed moving lib80211_crypt_ccmp.c into that portion of the series (which I haven't posted nor finished -- it's lower priority than fixing the cross-field writes).
In which case I'd probably prefer to do this separately from the staging drivers ...
Agreed. Sorry for the noise on that part. I will double-check the other patches.