* [PATCH v8 2/6] wifi: mac80211: Use struct instead of macro for PREP frame
2026-05-21 22:58 [PATCH v8 1/6] wifi: mac80211: Use struct instead of macro for PREQ frame Masashi Honma
@ 2026-05-21 22:58 ` Masashi Honma
2026-05-21 22:58 ` [PATCH v8 3/6] wifi: mac80211: Use struct instead of macro for PERR frame Masashi Honma
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Masashi Honma @ 2026-05-21 22:58 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes, Masashi Honma
The existing PREP_IE_* macros access HWMP PREP frame fields via hardcoded
byte offsets. When the AE (Address Extension) flag is set, an additional
6 bytes appear mid-frame, making the offset arithmetic error-prone.
Introduce typed packed C structs to represent the PREP frame layout:
- ieee80211_mesh_hwmp_prep_top: fixed fields before the optional AE
address
- ieee80211_mesh_hwmp_prep_bottom: fields after the optional AE address
Add ieee80211_mesh_hwmp_prep_get_bottom() to locate the bottom struct
correctly based on whether the AE flag is set.
This preparatory refactoring is needed to fix a 2-byte overread of
orig_addr in hwmp_prep_frame_process() when AE is enabled, which is
addressed in a subsequent patch.
Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
include/linux/ieee80211-mesh.h | 27 ++++++++++++++++++++
net/mac80211/mesh_hwmp.c | 46 ++++++++++++++++------------------
2 files changed, 49 insertions(+), 24 deletions(-)
diff --git a/include/linux/ieee80211-mesh.h b/include/linux/ieee80211-mesh.h
index bf4a544aed00..4ce4e47d6d01 100644
--- a/include/linux/ieee80211-mesh.h
+++ b/include/linux/ieee80211-mesh.h
@@ -53,6 +53,24 @@ struct ieee80211_mesh_hwmp_preq_bottom {
struct ieee80211_mesh_hwmp_preq_target targets[];
} __packed;
+struct ieee80211_mesh_hwmp_prep_top {
+ u8 flags;
+ u8 hopcount;
+ u8 ttl;
+ u8 target_addr[ETH_ALEN];
+ __le32 target_sn;
+
+ /* optional Target External Address */
+ u8 variable[];
+} __packed;
+
+struct ieee80211_mesh_hwmp_prep_bottom {
+ __le32 lifetime;
+ __le32 metric;
+ u8 orig_addr[ETH_ALEN];
+ __le32 orig_sn;
+} __packed;
+
/* Mesh flags */
#define MESH_FLAGS_AE_A4 0x1
#define MESH_FLAGS_AE_A5_A6 0x2
@@ -269,4 +287,13 @@ ieee80211_mesh_hwmp_preq_get_bottom(const u8 *ie)
ieee80211_mesh_preq_prep_ae_enabled(ie) ? ETH_ALEN : 0];
}
+static inline struct ieee80211_mesh_hwmp_prep_bottom *
+ieee80211_mesh_hwmp_prep_get_bottom(const u8 *ie)
+{
+ struct ieee80211_mesh_hwmp_prep_top *top = (void *)ie;
+
+ return (void *)&top->variable[
+ ieee80211_mesh_preq_prep_ae_enabled(ie) ? ETH_ALEN : 0];
+}
+
#endif /* LINUX_IEEE80211_MESH_H */
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 1a6a22b185d9..39b782370df0 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -37,16 +37,6 @@ static inline u16 u16_field_get(const u8 *preq_elem, int offset, bool ae)
/* HWMP IE processing macros */
#define AE_F_SET(x) (*x & AE_F)
-#define PREP_IE_FLAGS(x) (*(x))
-#define PREP_IE_HOPCOUNT(x) (*(x + 1))
-#define PREP_IE_TTL(x) (*(x + 2))
-#define PREP_IE_ORIG_ADDR(x) (AE_F_SET(x) ? x + 27 : x + 21)
-#define PREP_IE_ORIG_SN(x) u32_field_get(x, 27, AE_F_SET(x))
-#define PREP_IE_LIFETIME(x) u32_field_get(x, 13, AE_F_SET(x))
-#define PREP_IE_METRIC(x) u32_field_get(x, 17, AE_F_SET(x))
-#define PREP_IE_TARGET_ADDR(x) (x + 3)
-#define PREP_IE_TARGET_SN(x) u32_field_get(x, 9, 0)
-
#define PERR_IE_TTL(x) (*(x))
#define PERR_IE_TARGET_FLAGS(x) (*(x + 2))
#define PERR_IE_TARGET_ADDR(x) (x + 3)
@@ -419,11 +409,16 @@ static u32 hwmp_route_info_get(struct ieee80211_sub_if_data *sdata,
* so that we can easily use a single function to gather path
* information from both PREQ and PREP frames.
*/
- orig_addr = PREP_IE_TARGET_ADDR(hwmp_ie);
- orig_sn = PREP_IE_TARGET_SN(hwmp_ie);
- orig_lifetime = PREP_IE_LIFETIME(hwmp_ie);
- orig_metric = PREP_IE_METRIC(hwmp_ie);
- hopcount = PREP_IE_HOPCOUNT(hwmp_ie) + 1;
+ struct ieee80211_mesh_hwmp_prep_top *prep_elem_top =
+ (void *)hwmp_ie;
+ struct ieee80211_mesh_hwmp_prep_bottom *prep_elem_bottom =
+ ieee80211_mesh_hwmp_prep_get_bottom(hwmp_ie);
+
+ orig_addr = prep_elem_top->target_addr;
+ orig_sn = le32_to_cpu(prep_elem_top->target_sn);
+ orig_lifetime = le32_to_cpu(prep_elem_bottom->lifetime);
+ orig_metric = le32_to_cpu(prep_elem_bottom->metric);
+ hopcount = prep_elem_top->hopcount + 1;
break;
default:
rcu_read_unlock();
@@ -714,6 +709,9 @@ static void hwmp_prep_frame_process(struct ieee80211_sub_if_data *sdata,
const u8 *prep_elem, u32 metric)
{
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
+ struct ieee80211_mesh_hwmp_prep_top *prep_elem_top = (void *)prep_elem;
+ struct ieee80211_mesh_hwmp_prep_bottom *prep_elem_bottom =
+ ieee80211_mesh_hwmp_prep_get_bottom(prep_elem);
struct mesh_path *mpath;
const u8 *target_addr, *orig_addr;
u8 ttl, hopcount, flags;
@@ -721,9 +719,9 @@ static void hwmp_prep_frame_process(struct ieee80211_sub_if_data *sdata,
u32 target_sn, orig_sn, lifetime;
mhwmp_dbg(sdata, "received PREP from %pM\n",
- PREP_IE_TARGET_ADDR(prep_elem));
+ prep_elem_top->target_addr);
- orig_addr = PREP_IE_ORIG_ADDR(prep_elem);
+ orig_addr = prep_elem_bottom->orig_addr;
if (ether_addr_equal(orig_addr, sdata->vif.addr))
/* destination, no forwarding required */
return;
@@ -731,7 +729,7 @@ static void hwmp_prep_frame_process(struct ieee80211_sub_if_data *sdata,
if (!ifmsh->mshcfg.dot11MeshForwarding)
return;
- ttl = PREP_IE_TTL(prep_elem);
+ ttl = prep_elem_top->ttl;
if (ttl <= 1) {
sdata->u.mesh.mshstats.dropped_frames_ttl++;
return;
@@ -750,12 +748,12 @@ static void hwmp_prep_frame_process(struct ieee80211_sub_if_data *sdata,
memcpy(next_hop, next_hop_deref_protected(mpath)->sta.addr, ETH_ALEN);
spin_unlock_bh(&mpath->state_lock);
--ttl;
- flags = PREP_IE_FLAGS(prep_elem);
- lifetime = PREP_IE_LIFETIME(prep_elem);
- hopcount = PREP_IE_HOPCOUNT(prep_elem) + 1;
- target_addr = PREP_IE_TARGET_ADDR(prep_elem);
- target_sn = PREP_IE_TARGET_SN(prep_elem);
- orig_sn = PREP_IE_ORIG_SN(prep_elem);
+ flags = prep_elem_top->flags;
+ lifetime = le32_to_cpu(prep_elem_bottom->lifetime);
+ hopcount = prep_elem_top->hopcount + 1;
+ target_addr = prep_elem_top->target_addr;
+ target_sn = le32_to_cpu(prep_elem_top->target_sn);
+ orig_sn = le32_to_cpu(prep_elem_bottom->orig_sn);
mesh_path_sel_frame_tx(MPATH_PREP, flags, orig_addr, orig_sn, 0,
target_addr, target_sn, next_hop, hopcount,
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v8 3/6] wifi: mac80211: Use struct instead of macro for PERR frame
2026-05-21 22:58 [PATCH v8 1/6] wifi: mac80211: Use struct instead of macro for PREQ frame Masashi Honma
2026-05-21 22:58 ` [PATCH v8 2/6] wifi: mac80211: Use struct instead of macro for PREP frame Masashi Honma
@ 2026-05-21 22:58 ` Masashi Honma
2026-05-28 8:00 ` Johannes Berg
2026-05-21 22:58 ` [PATCH v8 4/6] wifi: mac80211: Fix overread in PREQ frame processing Masashi Honma
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Masashi Honma @ 2026-05-21 22:58 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes, Masashi Honma
The existing PERR_IE_* macros access HWMP PERR frame fields via hardcoded
byte offsets. Each PERR destination entry contains an optional 6-byte AE
(Address Extension) address followed by a reason code, making offset-based
access error-prone.
Introduce typed packed C structs to represent the PERR frame layout:
- ieee80211_mesh_hwmp_perr: top-level frame containing TTL and
destination count
- ieee80211_mesh_hwmp_perr_dst: per-destination entry with optional AE
address and variable-position reason code
Add ieee80211_mesh_hwmp_perr_get_rcode() to locate the reason code in
each destination entry depending on whether the AE flag is set.
This refactoring makes the PERR processing code consistent with the
struct-based approach adopted for PREQ and PREP in preceding patches.
Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
include/linux/ieee80211-mesh.h | 25 +++++++++++++++++++++++++
net/mac80211/mesh_hwmp.c | 31 +++++--------------------------
2 files changed, 30 insertions(+), 26 deletions(-)
diff --git a/include/linux/ieee80211-mesh.h b/include/linux/ieee80211-mesh.h
index 4ce4e47d6d01..0e9bd56b54f2 100644
--- a/include/linux/ieee80211-mesh.h
+++ b/include/linux/ieee80211-mesh.h
@@ -71,6 +71,20 @@ struct ieee80211_mesh_hwmp_prep_bottom {
__le32 orig_sn;
} __packed;
+struct ieee80211_mesh_hwmp_perr_dst {
+ u8 flags;
+ u8 addr[ETH_ALEN];
+ __le32 sn;
+ /* optional Destination External Address */
+ u8 variable[];
+} __packed;
+
+struct ieee80211_mesh_hwmp_perr {
+ u8 ttl;
+ u8 number_of_dst;
+ struct ieee80211_mesh_hwmp_perr_dst dsts[];
+} __packed;
+
/* Mesh flags */
#define MESH_FLAGS_AE_A4 0x1
#define MESH_FLAGS_AE_A5_A6 0x2
@@ -296,4 +310,15 @@ ieee80211_mesh_hwmp_prep_get_bottom(const u8 *ie)
ieee80211_mesh_preq_prep_ae_enabled(ie) ? ETH_ALEN : 0];
}
+static inline u16
+ieee80211_mesh_hwmp_perr_get_rcode(const u8 *ie, u8 dst_idx)
+{
+ struct ieee80211_mesh_hwmp_perr *perr_ie = (void *)ie;
+ struct ieee80211_mesh_hwmp_perr_dst *dst =
+ &perr_ie->dsts[dst_idx];
+
+ return get_unaligned_le16(&dst->variable[
+ (dst->flags & AE_F) ? ETH_ALEN : 0]);
+}
+
#endif /* LINUX_IEEE80211_MESH_H */
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 39b782370df0..fa144a187fe2 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -20,29 +20,7 @@
static void mesh_queue_preq(struct mesh_path *, u8);
-static inline u32 u32_field_get(const u8 *preq_elem, int offset, bool ae)
-{
- if (ae)
- offset += 6;
- return get_unaligned_le32(preq_elem + offset);
-}
-
-static inline u16 u16_field_get(const u8 *preq_elem, int offset, bool ae)
-{
- if (ae)
- offset += 6;
- return get_unaligned_le16(preq_elem + offset);
-}
-
/* HWMP IE processing macros */
-#define AE_F_SET(x) (*x & AE_F)
-
-#define PERR_IE_TTL(x) (*(x))
-#define PERR_IE_TARGET_FLAGS(x) (*(x + 2))
-#define PERR_IE_TARGET_ADDR(x) (x + 3)
-#define PERR_IE_TARGET_SN(x) u32_field_get(x, 9, 0)
-#define PERR_IE_TARGET_RCODE(x) u16_field_get(x, 13, 0)
-
#define MSEC_TO_TU(x) (x*1000/1024)
#define SN_GT(x, y) ((s32)(y - x) < 0)
#define SN_LT(x, y) ((s32)(x - y) < 0)
@@ -774,6 +752,7 @@ static void hwmp_perr_frame_process(struct ieee80211_sub_if_data *sdata,
const u8 *perr_elem)
{
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
+ struct ieee80211_mesh_hwmp_perr *perr_elem_s = (void *)perr_elem;
struct mesh_path *mpath;
u8 ttl;
const u8 *ta, *target_addr;
@@ -781,15 +760,15 @@ static void hwmp_perr_frame_process(struct ieee80211_sub_if_data *sdata,
u16 target_rcode;
ta = mgmt->sa;
- ttl = PERR_IE_TTL(perr_elem);
+ ttl = perr_elem_s->ttl;
if (ttl <= 1) {
ifmsh->mshstats.dropped_frames_ttl++;
return;
}
ttl--;
- target_addr = PERR_IE_TARGET_ADDR(perr_elem);
- target_sn = PERR_IE_TARGET_SN(perr_elem);
- target_rcode = PERR_IE_TARGET_RCODE(perr_elem);
+ target_addr = perr_elem_s->dsts[0].addr;
+ target_sn = le32_to_cpu(perr_elem_s->dsts[0].sn);
+ target_rcode = ieee80211_mesh_hwmp_perr_get_rcode(perr_elem, 0);
rcu_read_lock();
mpath = mesh_path_lookup(sdata, target_addr);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v8 3/6] wifi: mac80211: Use struct instead of macro for PERR frame
2026-05-21 22:58 ` [PATCH v8 3/6] wifi: mac80211: Use struct instead of macro for PERR frame Masashi Honma
@ 2026-05-28 8:00 ` Johannes Berg
2026-05-29 23:06 ` Masashi Honma
0 siblings, 1 reply; 12+ messages in thread
From: Johannes Berg @ 2026-05-28 8:00 UTC (permalink / raw)
To: Masashi Honma, linux-wireless
Hi,
So I was just about to apply this set, but now I'm thinking about this:
> +struct ieee80211_mesh_hwmp_perr_dst {
> + u8 flags;
> + u8 addr[ETH_ALEN];
> + __le32 sn;
> + /* optional Destination External Address */
> + u8 variable[];
> +} __packed;
This has a variable member, and the presence of the address in
'variable' depends on the flags (AE_F set there.)
> +struct ieee80211_mesh_hwmp_perr {
> + u8 ttl;
> + u8 number_of_dst;
> + struct ieee80211_mesh_hwmp_perr_dst dsts[];
> +} __packed;
However this declares an array of these, and that doesn't really seem
right. It effectively puts non-variable fields (e.g. dsts[1].ttl) after
the variable field dsts[0].variable. I _think_ some compilers (versions)
will also (rightfully) complain about this.
It seems like this should just be
struct ... {
u8 ttl;
u8 number_of_dst;
/* list of variably sized struct ieee80211_mesh_hwmp_perr_dst
*/
u8 dsts[];
} __packed;
> +static inline u16
> +ieee80211_mesh_hwmp_perr_get_rcode(const u8 *ie, u8 dst_idx)
> +{
> + struct ieee80211_mesh_hwmp_perr *perr_ie = (void *)ie;
> + struct ieee80211_mesh_hwmp_perr_dst *dst =
> + &perr_ie->dsts[dst_idx];
And especially this indexing doesn't seem like it could work - you have
to walk through all of them to see if each has the AE_F set and skip
sizeof() + optional ETH_ALEN.
> +
> + return get_unaligned_le16(&dst->variable[
> + (dst->flags & AE_F) ? ETH_ALEN : 0]);
This looks like the comment above should be
/* optional Destination External Address, rcode */
u8 variable[];
or so?
> + target_rcode = ieee80211_mesh_hwmp_perr_get_rcode(perr_elem, 0);
but evidently, this doesn't really matter right now if only one
destination entry is ever read.
Still, please fix that, if only to avoid the compiler warnings I'm
imagining will happen.
johannes
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v8 3/6] wifi: mac80211: Use struct instead of macro for PERR frame
2026-05-28 8:00 ` Johannes Berg
@ 2026-05-29 23:06 ` Masashi Honma
0 siblings, 0 replies; 12+ messages in thread
From: Masashi Honma @ 2026-05-29 23:06 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
Thank you for your reviews!
Ah sorry, I overlooked this point, so I have fixed it. I also added
KUnit, as the code was becoming complex.
2026年5月28日(木) 17:00 Johannes Berg <johannes@sipsolutions.net>:
>
> Hi,
>
> So I was just about to apply this set, but now I'm thinking about this:
>
>
> > +struct ieee80211_mesh_hwmp_perr_dst {
> > + u8 flags;
> > + u8 addr[ETH_ALEN];
> > + __le32 sn;
> > + /* optional Destination External Address */
> > + u8 variable[];
> > +} __packed;
>
> This has a variable member, and the presence of the address in
> 'variable' depends on the flags (AE_F set there.)
>
> > +struct ieee80211_mesh_hwmp_perr {
> > + u8 ttl;
> > + u8 number_of_dst;
> > + struct ieee80211_mesh_hwmp_perr_dst dsts[];
> > +} __packed;
>
> However this declares an array of these, and that doesn't really seem
> right. It effectively puts non-variable fields (e.g. dsts[1].ttl) after
> the variable field dsts[0].variable. I _think_ some compilers (versions)
> will also (rightfully) complain about this.
>
> It seems like this should just be
>
> struct ... {
> u8 ttl;
> u8 number_of_dst;
>
> /* list of variably sized struct ieee80211_mesh_hwmp_perr_dst
> */
> u8 dsts[];
> } __packed;
>
> > +static inline u16
> > +ieee80211_mesh_hwmp_perr_get_rcode(const u8 *ie, u8 dst_idx)
> > +{
> > + struct ieee80211_mesh_hwmp_perr *perr_ie = (void *)ie;
> > + struct ieee80211_mesh_hwmp_perr_dst *dst =
> > + &perr_ie->dsts[dst_idx];
>
> And especially this indexing doesn't seem like it could work - you have
> to walk through all of them to see if each has the AE_F set and skip
> sizeof() + optional ETH_ALEN.
>
> > +
> > + return get_unaligned_le16(&dst->variable[
> > + (dst->flags & AE_F) ? ETH_ALEN : 0]);
>
> This looks like the comment above should be
>
> /* optional Destination External Address, rcode */
> u8 variable[];
>
> or so?
>
> > + target_rcode = ieee80211_mesh_hwmp_perr_get_rcode(perr_elem, 0);
>
> but evidently, this doesn't really matter right now if only one
> destination entry is ever read.
>
> Still, please fix that, if only to avoid the compiler warnings I'm
> imagining will happen.
>
> johannes
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v8 4/6] wifi: mac80211: Fix overread in PREQ frame processing
2026-05-21 22:58 [PATCH v8 1/6] wifi: mac80211: Use struct instead of macro for PREQ frame Masashi Honma
2026-05-21 22:58 ` [PATCH v8 2/6] wifi: mac80211: Use struct instead of macro for PREP frame Masashi Honma
2026-05-21 22:58 ` [PATCH v8 3/6] wifi: mac80211: Use struct instead of macro for PERR frame Masashi Honma
@ 2026-05-21 22:58 ` Masashi Honma
2026-05-28 8:04 ` Johannes Berg
2026-05-21 22:58 ` [PATCH v8 5/6] wifi: mac80211: Fix overread in PREP " Masashi Honma
2026-05-21 22:58 ` [PATCH v8 6/6] wifi: mac80211: Fix PERR " Masashi Honma
4 siblings, 1 reply; 12+ messages in thread
From: Masashi Honma @ 2026-05-21 22:58 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes, Masashi Honma
When the AF flag is enabled, hwmp_preq_frame_process() overreads
target_addr by 2 bytes. Since this occurs within the socket buffer, it does
not read across memory boundaries and therefore poses no security risk;
however, we will fix it as a precaution.
In this fix, a new function mesh_path_parse_request_frame() is established
to separate the implementation of frame format validation and the check for
unsupported features. This is intended to facilitate future work when
implementing the currently unsupported parts.
Assisted-by: Claude:Sonnet 4.6
Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
include/linux/ieee80211-mesh.h | 28 ++++++++++++++++++++++++++++
net/mac80211/mesh_hwmp.c | 12 ++++++++++--
net/mac80211/parse.c | 9 +++++++--
3 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/include/linux/ieee80211-mesh.h b/include/linux/ieee80211-mesh.h
index 0e9bd56b54f2..42a5bd73838c 100644
--- a/include/linux/ieee80211-mesh.h
+++ b/include/linux/ieee80211-mesh.h
@@ -321,4 +321,32 @@ ieee80211_mesh_hwmp_perr_get_rcode(const u8 *ie, u8 dst_idx)
(dst->flags & AE_F) ? ETH_ALEN : 0]);
}
+/* IEEE Std 802.11-2016 9.4.2.113 PREQ element */
+static inline bool ieee80211_mesh_preq_size_ok(const u8 *pos, u8 elen)
+{
+ struct ieee80211_mesh_hwmp_preq_bottom *preq_elem_bottom =
+ ieee80211_mesh_hwmp_preq_get_bottom(pos);
+ u8 target_count;
+ u8 needed;
+
+ /* Check if the element contains flags */
+ if (elen < 1)
+ return false;
+
+ /* Check if the element contains target_count */
+ needed = sizeof(struct ieee80211_mesh_hwmp_preq_top) +
+ (ieee80211_mesh_preq_prep_ae_enabled(pos) ? ETH_ALEN : 0)
+ /* Originator External Address */ +
+ sizeof(struct ieee80211_mesh_hwmp_preq_bottom);
+ if (elen < needed)
+ return false;
+
+ target_count = preq_elem_bottom->target_count;
+ if (target_count < 1 || target_count > 20)
+ return false;
+
+ needed += target_count * sizeof(struct ieee80211_mesh_hwmp_preq_target);
+ return elen == needed;
+}
+
#endif /* LINUX_IEEE80211_MESH_H */
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index fa144a187fe2..ad3e575a0a94 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -929,9 +929,17 @@ void mesh_rx_path_sel_frame(struct ieee80211_sub_if_data *sdata,
return;
if (elems->preq) {
- if (elems->preq_len != 37)
- /* Right now we support just 1 destination and no AE */
+ struct ieee80211_mesh_hwmp_preq_bottom *preq_elem_bottom =
+ ieee80211_mesh_hwmp_preq_get_bottom(elems->preq);
+
+ /* Right now we do not support AE (Address Extension) */
+ if (ieee80211_mesh_preq_prep_ae_enabled(elems->preq))
goto free;
+
+ /* Right now we only support 1 target */
+ if (preq_elem_bottom->target_count != 1)
+ goto free;
+
path_metric = hwmp_route_info_get(sdata, mgmt, elems->preq,
MPATH_PREQ);
if (path_metric)
diff --git a/net/mac80211/parse.c b/net/mac80211/parse.c
index 5e61457be0f3..9e52cc48fc18 100644
--- a/net/mac80211/parse.c
+++ b/net/mac80211/parse.c
@@ -547,8 +547,13 @@ _ieee802_11_parse_elems_full(struct ieee80211_elems_parse_params *params,
elems->awake_window = (void *)pos;
break;
case WLAN_EID_PREQ:
- elems->preq = pos;
- elems->preq_len = elen;
+ if (ieee80211_mesh_preq_size_ok(pos, elen)) {
+ elems->preq = pos;
+ elems->preq_len = elen;
+ } else {
+ elem_parse_failed =
+ IEEE80211_PARSE_ERR_BAD_ELEM_SIZE;
+ }
break;
case WLAN_EID_PREP:
elems->prep = pos;
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v8 4/6] wifi: mac80211: Fix overread in PREQ frame processing
2026-05-21 22:58 ` [PATCH v8 4/6] wifi: mac80211: Fix overread in PREQ frame processing Masashi Honma
@ 2026-05-28 8:04 ` Johannes Berg
2026-05-29 23:06 ` Masashi Honma
0 siblings, 1 reply; 12+ messages in thread
From: Johannes Berg @ 2026-05-28 8:04 UTC (permalink / raw)
To: Masashi Honma, linux-wireless
On Fri, 2026-05-22 at 07:58 +0900, Masashi Honma wrote:
>
> +/* IEEE Std 802.11-2016 9.4.2.113 PREQ element */
> +static inline bool ieee80211_mesh_preq_size_ok(const u8 *pos, u8 elen)
> +{
> + struct ieee80211_mesh_hwmp_preq_bottom *preq_elem_bottom =
> + ieee80211_mesh_hwmp_preq_get_bottom(pos);
> + u8 target_count;
> + u8 needed;
> +
> + /* Check if the element contains flags */
> + if (elen < 1)
> + return false;
> +
> + /* Check if the element contains target_count */
> + needed = sizeof(struct ieee80211_mesh_hwmp_preq_top) +
> + (ieee80211_mesh_preq_prep_ae_enabled(pos) ? ETH_ALEN : 0)
> + /* Originator External Address */ +
> + sizeof(struct ieee80211_mesh_hwmp_preq_bottom);
> + if (elen < needed)
> + return false;
> +
> + target_count = preq_elem_bottom->target_count;
> + if (target_count < 1 || target_count > 20)
> + return false;
While this is correct now, I think it's perhaps too tricky ...
The reason it's correct is that needed starts out with at most
sizeof(top) + ETH_ALEN + sizeof(bottom) == 17 + 6 + 9 == 32, target
sizeof is 11, so 20*11+32 == 252 and cannot overflow.
But I think it'd be far simpler to declare "needed" simply as 'int',
then even with target_count == 255 and whatever else happened before
cannot overflow, the elen==needed check will promote to int and refuse a
bad target_count implicitly instead of needing to do so explicitly to
avoid the integer overflow.
johannes
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v8 4/6] wifi: mac80211: Fix overread in PREQ frame processing
2026-05-28 8:04 ` Johannes Berg
@ 2026-05-29 23:06 ` Masashi Honma
0 siblings, 0 replies; 12+ messages in thread
From: Masashi Honma @ 2026-05-29 23:06 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
Yes, fixed.
2026年5月28日(木) 17:04 Johannes Berg <johannes@sipsolutions.net>:
>
> On Fri, 2026-05-22 at 07:58 +0900, Masashi Honma wrote:
> >
> > +/* IEEE Std 802.11-2016 9.4.2.113 PREQ element */
> > +static inline bool ieee80211_mesh_preq_size_ok(const u8 *pos, u8 elen)
> > +{
> > + struct ieee80211_mesh_hwmp_preq_bottom *preq_elem_bottom =
> > + ieee80211_mesh_hwmp_preq_get_bottom(pos);
> > + u8 target_count;
> > + u8 needed;
> > +
> > + /* Check if the element contains flags */
> > + if (elen < 1)
> > + return false;
> > +
> > + /* Check if the element contains target_count */
> > + needed = sizeof(struct ieee80211_mesh_hwmp_preq_top) +
> > + (ieee80211_mesh_preq_prep_ae_enabled(pos) ? ETH_ALEN : 0)
> > + /* Originator External Address */ +
> > + sizeof(struct ieee80211_mesh_hwmp_preq_bottom);
> > + if (elen < needed)
> > + return false;
> > +
> > + target_count = preq_elem_bottom->target_count;
> > + if (target_count < 1 || target_count > 20)
> > + return false;
>
> While this is correct now, I think it's perhaps too tricky ...
>
> The reason it's correct is that needed starts out with at most
> sizeof(top) + ETH_ALEN + sizeof(bottom) == 17 + 6 + 9 == 32, target
> sizeof is 11, so 20*11+32 == 252 and cannot overflow.
>
> But I think it'd be far simpler to declare "needed" simply as 'int',
> then even with target_count == 255 and whatever else happened before
> cannot overflow, the elen==needed check will promote to int and refuse a
> bad target_count implicitly instead of needing to do so explicitly to
> avoid the integer overflow.
>
> johannes
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v8 5/6] wifi: mac80211: Fix overread in PREP frame processing
2026-05-21 22:58 [PATCH v8 1/6] wifi: mac80211: Use struct instead of macro for PREQ frame Masashi Honma
` (2 preceding siblings ...)
2026-05-21 22:58 ` [PATCH v8 4/6] wifi: mac80211: Fix overread in PREQ frame processing Masashi Honma
@ 2026-05-21 22:58 ` Masashi Honma
2026-05-21 22:58 ` [PATCH v8 6/6] wifi: mac80211: Fix PERR " Masashi Honma
4 siblings, 0 replies; 12+ messages in thread
From: Masashi Honma @ 2026-05-21 22:58 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes, Masashi Honma
When the AF flag is enabled, hwmp_prep_frame_process() overreads orig_addr
by 2 bytes. Since this occurs within the socket buffer, it does not read
across memory boundaries and therefore poses no security risk; however, we
will fix it as a precaution.
In this fix, a new function mesh_path_parse_reply_frame() is established to
separate the implementation of frame format validation and the check for
unsupported features. This is intended to facilitate future work when
implementing the currently unsupported parts.
Assisted-by: Claude:Sonnet 4.6
Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
include/linux/ieee80211-mesh.h | 16 ++++++++++++++++
net/mac80211/mesh_hwmp.c | 4 ++--
net/mac80211/parse.c | 9 +++++++--
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/include/linux/ieee80211-mesh.h b/include/linux/ieee80211-mesh.h
index 42a5bd73838c..b4fca2937de0 100644
--- a/include/linux/ieee80211-mesh.h
+++ b/include/linux/ieee80211-mesh.h
@@ -349,4 +349,20 @@ static inline bool ieee80211_mesh_preq_size_ok(const u8 *pos, u8 elen)
return elen == needed;
}
+/* IEEE Std 802.11-2016 9.4.2.114 PREP element */
+static inline bool ieee80211_mesh_prep_size_ok(const u8 *pos, u8 elen)
+{
+ u8 needed;
+
+ /* Check if the element contains flags */
+ if (elen < 1)
+ return false;
+
+ needed = sizeof(struct ieee80211_mesh_hwmp_prep_top) +
+ (ieee80211_mesh_preq_prep_ae_enabled(pos) ? ETH_ALEN : 0)
+ /* Target External Address */ +
+ sizeof(struct ieee80211_mesh_hwmp_prep_bottom);
+ return elen == needed;
+}
+
#endif /* LINUX_IEEE80211_MESH_H */
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index ad3e575a0a94..391d37721b23 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -947,8 +947,8 @@ void mesh_rx_path_sel_frame(struct ieee80211_sub_if_data *sdata,
path_metric);
}
if (elems->prep) {
- if (elems->prep_len != 31)
- /* Right now we support no AE */
+ /* Right now we do not support AE (Address Extension) */
+ if (ieee80211_mesh_preq_prep_ae_enabled(elems->prep))
goto free;
path_metric = hwmp_route_info_get(sdata, mgmt, elems->prep,
MPATH_PREP);
diff --git a/net/mac80211/parse.c b/net/mac80211/parse.c
index 9e52cc48fc18..bbd1e1bc77b4 100644
--- a/net/mac80211/parse.c
+++ b/net/mac80211/parse.c
@@ -556,8 +556,13 @@ _ieee802_11_parse_elems_full(struct ieee80211_elems_parse_params *params,
}
break;
case WLAN_EID_PREP:
- elems->prep = pos;
- elems->prep_len = elen;
+ if (ieee80211_mesh_prep_size_ok(pos, elen)) {
+ elems->prep = pos;
+ elems->prep_len = elen;
+ } else {
+ elem_parse_failed =
+ IEEE80211_PARSE_ERR_BAD_ELEM_SIZE;
+ }
break;
case WLAN_EID_PERR:
elems->perr = pos;
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v8 6/6] wifi: mac80211: Fix PERR frame processing
2026-05-21 22:58 [PATCH v8 1/6] wifi: mac80211: Use struct instead of macro for PREQ frame Masashi Honma
` (3 preceding siblings ...)
2026-05-21 22:58 ` [PATCH v8 5/6] wifi: mac80211: Fix overread in PREP " Masashi Honma
@ 2026-05-21 22:58 ` Masashi Honma
2026-05-28 8:12 ` Johannes Berg
4 siblings, 1 reply; 12+ messages in thread
From: Masashi Honma @ 2026-05-21 22:58 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes, Masashi Honma
There are no issues with the PERR processing itself; however, to maintain
consistency with the previous PREQ/PREP code modifications, I will create a
new mesh_path_parse_error_frame() function to separately implement the
frame format validation and the "not supported" check.
Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
include/linux/ieee80211-mesh.h | 41 ++++++++++++++++++++++++++++++++++
net/mac80211/mesh_hwmp.c | 14 ++++++++++--
net/mac80211/parse.c | 9 ++++++--
3 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/include/linux/ieee80211-mesh.h b/include/linux/ieee80211-mesh.h
index b4fca2937de0..bd83c647a578 100644
--- a/include/linux/ieee80211-mesh.h
+++ b/include/linux/ieee80211-mesh.h
@@ -365,4 +365,45 @@ static inline bool ieee80211_mesh_prep_size_ok(const u8 *pos, u8 elen)
return elen == needed;
}
+/* IEEE Std 802.11-2016 9.4.2.115 PERR element */
+static inline bool ieee80211_mesh_perr_size_ok(const u8 *pos, u8 elen)
+{
+ struct ieee80211_mesh_hwmp_perr *perr_elem = (void *)pos;
+ u8 number_of_dst;
+ u8 needed;
+ const u8 *start;
+ int i;
+
+ start = pos;
+ needed = sizeof(struct ieee80211_mesh_hwmp_perr);
+ pos += sizeof(struct ieee80211_mesh_hwmp_perr);
+
+ /* Check if the element contains number of dst */
+ if (elen < needed)
+ return false;
+
+ number_of_dst = perr_elem->number_of_dst;
+ if (number_of_dst < 1 || number_of_dst > 19)
+ return false;
+
+ for (i = 0; i < number_of_dst; i++) {
+ struct ieee80211_mesh_hwmp_perr_dst *perr_dst =
+ &perr_elem->dsts[i];
+ u8 dst_len;
+
+ /* Check if the element contains flags */
+ if (elen < pos - start + 1)
+ return false;
+
+ dst_len = sizeof(struct ieee80211_mesh_hwmp_perr_dst) +
+ ((perr_dst->flags & AE_F) ? ETH_ALEN : 0)
+ /* Destination External Address */ +
+ 2 /* Reason Code */;
+ needed += dst_len;
+ pos += dst_len;
+ }
+
+ return elen == needed;
+}
+
#endif /* LINUX_IEEE80211_MESH_H */
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 391d37721b23..a74d7b28a35d 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -957,9 +957,19 @@ void mesh_rx_path_sel_frame(struct ieee80211_sub_if_data *sdata,
path_metric);
}
if (elems->perr) {
- if (elems->perr_len != 15)
- /* Right now we support only one destination per PERR */
+ struct ieee80211_mesh_hwmp_perr *perr_elem =
+ (struct ieee80211_mesh_hwmp_perr *)elems->perr;
+ int i;
+
+ /* Right now we support only one destination per PERR */
+ if (perr_elem->number_of_dst != 1)
goto free;
+
+ /* Right now we do not support AE (Address Extension) */
+ for (i = 0; i < perr_elem->number_of_dst; i++)
+ if (perr_elem->dsts[i].flags & AE_F)
+ goto free;
+
hwmp_perr_frame_process(sdata, mgmt, elems->perr);
}
if (elems->rann)
diff --git a/net/mac80211/parse.c b/net/mac80211/parse.c
index bbd1e1bc77b4..d84e5e12ad24 100644
--- a/net/mac80211/parse.c
+++ b/net/mac80211/parse.c
@@ -565,8 +565,13 @@ _ieee802_11_parse_elems_full(struct ieee80211_elems_parse_params *params,
}
break;
case WLAN_EID_PERR:
- elems->perr = pos;
- elems->perr_len = elen;
+ if (ieee80211_mesh_perr_size_ok(pos, elen)) {
+ elems->perr = pos;
+ elems->perr_len = elen;
+ } else {
+ elem_parse_failed =
+ IEEE80211_PARSE_ERR_BAD_ELEM_SIZE;
+ }
break;
case WLAN_EID_RANN:
if (elen >= sizeof(struct ieee80211_rann_ie))
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v8 6/6] wifi: mac80211: Fix PERR frame processing
2026-05-21 22:58 ` [PATCH v8 6/6] wifi: mac80211: Fix PERR " Masashi Honma
@ 2026-05-28 8:12 ` Johannes Berg
2026-05-29 23:07 ` Masashi Honma
0 siblings, 1 reply; 12+ messages in thread
From: Johannes Berg @ 2026-05-28 8:12 UTC (permalink / raw)
To: Masashi Honma, linux-wireless
On Fri, 2026-05-22 at 07:58 +0900, Masashi Honma wrote:
>
> +/* IEEE Std 802.11-2016 9.4.2.115 PERR element */
> +static inline bool ieee80211_mesh_perr_size_ok(const u8 *pos, u8 elen)
> +{
> + struct ieee80211_mesh_hwmp_perr *perr_elem = (void *)pos;
> + u8 number_of_dst;
> + u8 needed;
> + const u8 *start;
> + int i;
> +
> + start = pos;
> + needed = sizeof(struct ieee80211_mesh_hwmp_perr);
> + pos += sizeof(struct ieee80211_mesh_hwmp_perr);
> +
> + /* Check if the element contains number of dst */
> + if (elen < needed)
> + return false;
> +
> + number_of_dst = perr_elem->number_of_dst;
> + if (number_of_dst < 1 || number_of_dst > 19)
> + return false;
Same here, though I didn't double-check this one; if we just go to 'int'
or 'unsigned int' for 'needed', it's not necessary to even have this.
> +
> + for (i = 0; i < number_of_dst; i++) {
> + struct ieee80211_mesh_hwmp_perr_dst *perr_dst =
> + &perr_elem->dsts[i];
> + u8 dst_len;
> +
> + /* Check if the element contains flags */
> + if (elen < pos - start + 1)
> + return false;
that comment seems a bit misleading. I figured out what you mean, but
IMHO it'd be more obvious if you wrote it as
for (...) {
struct _perr_dst *perr_dst;
u8 dst_len;
if (elen < pos - start + sizeof(*perr_dst))
return false;
> + dst_len = sizeof(struct ieee80211_mesh_hwmp_perr_dst) +
> + ((perr_dst->flags & AE_F) ? ETH_ALEN : 0)
> + /* Destination External Address */ +
> + 2 /* Reason Code */;
> + needed += dst_len;
> + pos += dst_len;
and technically that pos+= could be UB, so it should be
if (elen < pos - start + dst_len)
return false;
pos += dst_len;
> + /* Right now we do not support AE (Address Extension) */
> + for (i = 0; i < perr_elem->number_of_dst; i++)
> + if (perr_elem->dsts[i].flags & AE_F)
> + goto free;
This code will need to change based on what I commented on patch 3 wrt.
the variable members, although we only really use [0] and allow for a
single entry anyway ...
Probably should have an inline that returns struct
ieee80211_mesh_hwmp_perr_dst based on the index, and then have
ieee80211_mesh_hwmp_perr_get_rcode() work on that pointer instead of the
element and index.
Thanks for sticking with this, it already looks really nice!
johannes
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v8 6/6] wifi: mac80211: Fix PERR frame processing
2026-05-28 8:12 ` Johannes Berg
@ 2026-05-29 23:07 ` Masashi Honma
0 siblings, 0 replies; 12+ messages in thread
From: Masashi Honma @ 2026-05-29 23:07 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
Fixed as well.
2026年5月28日(木) 17:12 Johannes Berg <johannes@sipsolutions.net>:
>
> On Fri, 2026-05-22 at 07:58 +0900, Masashi Honma wrote:
> >
> > +/* IEEE Std 802.11-2016 9.4.2.115 PERR element */
> > +static inline bool ieee80211_mesh_perr_size_ok(const u8 *pos, u8 elen)
> > +{
> > + struct ieee80211_mesh_hwmp_perr *perr_elem = (void *)pos;
> > + u8 number_of_dst;
> > + u8 needed;
> > + const u8 *start;
> > + int i;
> > +
> > + start = pos;
> > + needed = sizeof(struct ieee80211_mesh_hwmp_perr);
> > + pos += sizeof(struct ieee80211_mesh_hwmp_perr);
> > +
> > + /* Check if the element contains number of dst */
> > + if (elen < needed)
> > + return false;
> > +
> > + number_of_dst = perr_elem->number_of_dst;
> > + if (number_of_dst < 1 || number_of_dst > 19)
> > + return false;
>
> Same here, though I didn't double-check this one; if we just go to 'int'
> or 'unsigned int' for 'needed', it's not necessary to even have this.
>
> > +
> > + for (i = 0; i < number_of_dst; i++) {
> > + struct ieee80211_mesh_hwmp_perr_dst *perr_dst =
> > + &perr_elem->dsts[i];
> > + u8 dst_len;
> > +
> > + /* Check if the element contains flags */
> > + if (elen < pos - start + 1)
> > + return false;
>
> that comment seems a bit misleading. I figured out what you mean, but
> IMHO it'd be more obvious if you wrote it as
>
> for (...) {
> struct _perr_dst *perr_dst;
> u8 dst_len;
>
> if (elen < pos - start + sizeof(*perr_dst))
> return false;
>
> > + dst_len = sizeof(struct ieee80211_mesh_hwmp_perr_dst) +
> > + ((perr_dst->flags & AE_F) ? ETH_ALEN : 0)
> > + /* Destination External Address */ +
> > + 2 /* Reason Code */;
> > + needed += dst_len;
> > + pos += dst_len;
>
> and technically that pos+= could be UB, so it should be
>
> if (elen < pos - start + dst_len)
> return false;
> pos += dst_len;
>
> > + /* Right now we do not support AE (Address Extension) */
> > + for (i = 0; i < perr_elem->number_of_dst; i++)
> > + if (perr_elem->dsts[i].flags & AE_F)
> > + goto free;
>
> This code will need to change based on what I commented on patch 3 wrt.
> the variable members, although we only really use [0] and allow for a
> single entry anyway ...
>
> Probably should have an inline that returns struct
> ieee80211_mesh_hwmp_perr_dst based on the index, and then have
> ieee80211_mesh_hwmp_perr_get_rcode() work on that pointer instead of the
> element and index.
>
>
> Thanks for sticking with this, it already looks really nice!
>
> johannes
^ permalink raw reply [flat|nested] 12+ messages in thread