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;
Message-ID: <82b75a5e344b23a388a8f078de730272e91f99d2.camel@sipsolutions.net>
Subject: Re: [PATCH] wifi: nl80211: fix mbssid nesting
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Koen Vandeputte <koen.vandeputte@citymesh.com>
Cc:     linux-wireless@vger.kernel.org, John Crispin <john@phrozen.org>,
        Aloka Dixit <alokad@codeaurora.org>, stable@vger.kernel.org
Date:   Wed, 12 Jul 2023 12:34:33 +0200
In-Reply-To: <CAPh3n82BY0bFkHs6DKKf6+ZUZw+DJmTy7=JEG8REFHyHFMqoVQ@mail.gmail.com>
References: <20230712083841.222607-1-koen.vandeputte@citymesh.com>
         <9936de52da43347ba0ccfb8737440a9698fb4585.camel@sipsolutions.net>
         <CAPh3n82BY0bFkHs6DKKf6+ZUZw+DJmTy7=JEG8REFHyHFMqoVQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

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:
> >=20
> > 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):
> > >=20
> > > GetWiphy: Type: 1
> > > GetWiphy: Type: 2
> > > GetWiphy: Type: 46
> > > GetWiphy: Type: 33074 <-- wrong enum value, above MAX also ..
> >=20
> > That's not wrong, that's just NLA_F_NESTED | NL80211_ATTR_MBSSID_CONFIG=
,
> > since it *is* in fact a nested attribute.
>=20
> ahha! so one should check on each received attribute if this flag is set?

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?

johannes