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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f97efe8ff5f466ce938f38dc26cb76c7abcc5a71.camel@sipsolutions.net>
Date:   Thu, 25 Aug 2022 10:08:40 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Gilad Itzkovitch <gilad.itzkovitch@...semicro.com>
Cc:     linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] wifi: ieee80211: Fix for fragmented action frames

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@...semicro.com>
> ---
>  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