linux-kernel - Re: Possible Null pointer dereferences in net/mac80211/parse.c

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] [day] [month] [year] [list]

Message-ID: <0b9d4a5d-326c-4281-b9fe-c8e1b1e4a026@oss.qualcomm.com>
Date: Mon, 10 Mar 2025 10:09:25 -0700
From: Jeff Johnson <jeff.johnson@....qualcomm.com>
To: Brahmajit <brahmajit.xyz@...il.com>,
        linux-kernel <linux-kernel@...r.kernel.org>
Cc: linux-wireless@...r.kernel.org, johannes.berg@...el.com,
        ilan.peer@...el.com, miriam.rachel.korenblit@...el.com
Subject: Re: Possible Null pointer dereferences in net/mac80211/parse.c

On 3/10/2025 7:16 AM, Brahmajit wrote:
> Coverity Scan reports that there might be a possible NULL pointer
> dereferences in net/mac80211/parse.c: 1061 in
> ieee802_11_parse_elems_full(). I understand that these reports are not
> always correct.
> 
> I'm not sure whether the syntax
> struct ieee80211_elems_parse_params sub = {};
> is correct or falls under C11 standard[0].

{} initializers are extensions supported by both gcc and clang/LLVM which
AFAIK are the only two compilers currently used by Linux.

> 
> initializer:
>          assignment-expression
>          { initializer-list }
>          { initializer-list , }
> initializer-list:
>          designation(opt) initializer
>          initializer-list , designation(opt) initializer
> 
> I'm aware that C23 allows empty initialization[1].
> 
> braced-initializer:
>                     { }
>                     { initializer-list }
>                     { initializer-list , }
> 
> Considering [0], if we do something like
> 
> --- a/net/mac80211/parse.c
> +++ b/net/mac80211/parse.c
> @@ -997,7 +997,7 @@ ieee80211_mle_defrag_epcs(struct ieee80211_elems_parse *elems_parse)
>  struct ieee802_11_elems *
>  ieee802_11_parse_elems_full(struct ieee80211_elems_parse_params *params)
>  {
> -       struct ieee80211_elems_parse_params sub = {};
> +       struct ieee80211_elems_parse_params sub = { 0 };

using one of the supported compilers, these are identical for this struct.

note that {} is usually preferable to {0} since {} works even when the first
member is not a scalar.

and the coverity report indicates the issue is that sub.start can be NULL,
which will initially be true with either of these initializers.

so the question is can sub.start really be NULL at the point where it is
passed to cfg80211_find_elem(), or does every path initialize it?

that is the question you should try to answer

>         struct ieee80211_elems_parse *elems_parse;
>         const struct element *non_inherit = NULL;
>         struct ieee802_11_elems *elems;
> 
> Would it be incorrect? Would appreciate some feedback.
> 
> [0]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
> [1]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3054.pdf