Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230712083841.222607-1-koen.vandeputte@citymesh.com>
 <9936de52da43347ba0ccfb8737440a9698fb4585.camel@sipsolutions.net>
 <CAPh3n82BY0bFkHs6DKKf6+ZUZw+DJmTy7=JEG8REFHyHFMqoVQ@mail.gmail.com> <82b75a5e344b23a388a8f078de730272e91f99d2.camel@sipsolutions.net>
In-Reply-To: <82b75a5e344b23a388a8f078de730272e91f99d2.camel@sipsolutions.net>
From:   Koen Vandeputte <koen.vandeputte@citymesh.com>
Date:   Wed, 12 Jul 2023 13:08:21 +0200
Message-ID: <CAPh3n82NPvU4AXgQ3y21kSkMWW18UNrOc1StKHuYeGRN4xNt5Q@mail.gmail.com>
Subject: Re: [PATCH] wifi: nl80211: fix mbssid nesting
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     linux-wireless@vger.kernel.org, John Crispin <john@phrozen.org>,
        Aloka Dixit <alokad@codeaurora.org>, stable@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Jul 12, 2023 at 12:34=E2=80=AFPM Johannes Berg
<johannes@sipsolutions.net> wrote:
>
> On Wed, 2023-07-12 at 11:05 +0200, Koen Vandeputte wrote:
> > On Wed, Jul 12, 2023 at 10:44=E2=80=AFAM Johannes Berg
> > <johannes@sipsolutions.net> wrote:
> > >
> > > On Wed, 2023-07-12 at 10:38 +0200, Koen Vandeputte wrote:
> > > > Executing command NL80211_CMD_GET_WIPHY and parsing it's output
> > > > natively without libnl shows following attributes as part of
> > > > the nl80211 generated netlink message (part 16):
> > > >
> > > > GetWiphy: Type: 1
> > > > GetWiphy: Type: 2
> > > > GetWiphy: Type: 46
> > > > GetWiphy: Type: 33074 <-- wrong enum value, above MAX also ..
> > >
> > > That's not wrong, that's just NLA_F_NESTED | NL80211_ATTR_MBSSID_CONF=
IG,
> > > since it *is* in fact a nested attribute.
> >
> > ahha! so one should check on each received attribute if this flag is se=
t?
>
> Yeah, that indicates that it's nested, without having the out-of-band
> information that it should be nested.
>
>
> There's also NLA_F_NET_BYTEORDER, so we really only have 14 bits for the
> type, so you should probably use NLA_TYPE_MASK.
>
> > I guess all the ones with the noflag so far are using it to avoid
> > breaking legacy stuff?
>
> Right. When we realized that nla_put_nested() didn't actually put the
> nested flag it was kind of too late - although a lot of userspace
> probably would have handled it correctly (if it uses libnl, etc.) But
> still, we converted it all to nla_put_nested_noflag(), but we really
> shouldn't be adding new code that calls nla_put_nested_noflag() since by
> definition new code is new attributes, and then userspace can be
> updated.
>
> Unless you're saying this somehow breaks old userspace that doesn't even
> understand NL80211_ATTR_MBSSID_CONFIG yet? But that seems unlikely, you
> have to be prepared to see attributes bigger than what you expected and
> skip them?
>
Yeah, I've already updated our internal code to filter netlink type
using NLA_TYPE_MASK.
Works like a charm now.
Our codebase was initially made during kernel 3.x time when that
filtering was not needed.. and within OpenWRT, the wireless stack got
updated to 6.1.x
bringing in mbssid netlink from 5.16.
So this popped up suddenly :-)

Thanks for the patience and sorry for the noise!

> johannes