Hi Johannes,
On Thu, Aug 25, 2022 at 8:08 PM Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:
>
> 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
That was partially our understanding. But, the fragmented action frame is
being dropped by this function as it is part of the provisioning DPP process
(fragmented due to S1G low rates).
Trying to avoid a big change here for this specific action category.
As defragmentation will occur later on in the process there should be a
safe way to avoid dropping the frame beforehand.
Gilad
