Re: [PATCH] wifi: ieee80211: Fix for fragmented action frames

From: Johannes Berg
Date: Thu Aug 25 2022 - 04:08:55 EST


On Thu, 2022-08-11 at 10:48 +1200, Gilad Itzkovitch wrote:
> The robust management frame check ensures a station exists for
> the frame before proceeding, but there are some action frame
> categories which don't require an existing station, and so the
> _ieee80211_is_robust_mgmt_frame function peeks into the
> action frame's payload to identify the category and filter them out.
>
> In some scenarios, e.g. DPP at S1G data rates, action frames
> can get fragmented. This commit adds an extra check to ensure
> we don't peek into the payload of fragmented frames beyond the
> first fragment.
>
> Signed-off-by: Gilad Itzkovitch <gilad.itzkovitch@xxxxxxxxxxxxxx>
> ---
> include/linux/ieee80211.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
> index 55e6f4ad0ca6..5da9608fdce3 100644
> --- a/include/linux/ieee80211.h
> +++ b/include/linux/ieee80211.h
> @@ -4124,6 +4124,7 @@ static inline bool _ieee80211_is_robust_mgmt_frame(struct ieee80211_hdr *hdr)
>
> if (ieee80211_is_action(hdr->frame_control)) {
> u8 *category;
> + u16 sc;
>
> /*
> * Action frames, excluding Public Action frames, are Robust
> @@ -4134,6 +4135,17 @@ static inline bool _ieee80211_is_robust_mgmt_frame(struct ieee80211_hdr *hdr)
> */
> if (ieee80211_has_protected(hdr->frame_control))
> return true;
> +
> + /*
> + * Some action frames do not have a STA associated with them,
> + * so we rule them out from the robust management frame check.
> + * The category is within the payload, so we only proceed if
> + * we're checking the first fragment.
> + */
> + sc = le16_to_cpu(hdr->seq_ctrl);
> + if (sc & IEEE80211_SCTL_FRAG)
> + return false;
>


This doesn't make much sense to me - why would it be allowed or
necessary to call this function on a frame that wasn't yet defragmented?

johannes