On Wed, Aug 30, 2023 at 01:22:37PM -0700, Jeff Johnson wrote:

On 8/30/2023 12:51 PM, Christian Lamparter wrote:

quoted

Hi,

On 8/29/23 15:29, Jeff Johnson wrote:

quoted

Currently struct ieee80211_tim_ie defines:
    u8 virtual_map[1];

Per the guidance in [1] change this to be a flexible array.

As a result of this change, adjust all related struct size tests to
account for the fact that the sizeof(struct ieee80211_tim_ie) now
accounts for the minimum size of the virtual_map.

[1] https://docs.kernel.org/process/deprecated.html#zero-length-and-one-element-arrays

Signed-off-by: Jeff Johnson <redacted>
---

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index bd2f6e19c357..4cdc2eb98f16 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h

@@ -961,7 +961,7 @@ struct ieee80211_tim_ie {

      u8 dtim_period;
      u8 bitmap_ctrl;
      /* variable size: 1 - 251 bytes */
-    u8 virtual_map[1];
+    u8 virtual_map[];
  } __packed;

Uhh, the 802.11 (my 2012 Version has this in) spec in
8.4.2.7 TIM Element demands this to be 1 - 251 bytes.
And this is why there's a comment above... With your
change this could be confusing. Would it be possible
to fix that somehow? Like in a anonymous union/group
with a flexible array and a u8?

Adding Kees to the discussion for any advice. Yes, the virtual_map must
contain at least one octet but may contain more than one. And to complicate
matters, the information that tells us how many octets are actually present
is found outside the struct; the TLV header that precedes the struct will
contain the length of the struct, and hence the length of the bitmap is that
size - 2 (the size of the dtim_period and bitmap_ctrl fields).

Bummer about the count variable being elsewhere, but we'll deal with
that later. :)

For the array declaration, though, yes, we can do a "minimum size 1" like
this:

	union {
		u8 required_byte;
		DECLARE_FLEX_ARRAY(u8, virtual_map);
	};

-- 
Kees Cook