linux-kernel - Re: [PATCH v2] genksyms: Fix enum consts from a reference affecting new values

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: <CAK7LNAQxXR64=GjMOnr_hpEqLWEZwH_XUkvkmp0xabryE8WG0Q@mail.gmail.com>
Date: Fri, 6 Jun 2025 04:46:52 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Petr Pavlu <petr.pavlu@...e.com>
Cc: Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>, linux-kbuild@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] genksyms: Fix enum consts from a reference affecting
 new values

On Tue, Jun 3, 2025 at 10:02 PM Petr Pavlu <petr.pavlu@...e.com> wrote:
>
> Enumeration constants read from a symbol reference file can incorrectly
> affect new enumeration constants parsed from an actual input file.
>
> Example:
>
>  $ cat test.c
>  enum { E_A, E_B, E_MAX };
>  struct bar { int mem[E_MAX]; };
>  int foo(struct bar *a) {}
>  __GENKSYMS_EXPORT_SYMBOL(foo);
>
>  $ cat test.c | ./scripts/genksyms/genksyms -T test.0.symtypes
>  #SYMVER foo 0x070d854d
>
>  $ cat test.0.symtypes
>  E#E_MAX 2
>  s#bar struct bar { int mem [ E#E_MAX ] ; }
>  foo int foo ( s#bar * )
>
>  $ cat test.c | ./scripts/genksyms/genksyms -T test.1.symtypes -r test.0.symtypes
>  <stdin>:4: warning: foo: modversion changed because of changes in enum constant E_MAX
>  #SYMVER foo 0x9c9dfd81
>
>  $ cat test.1.symtypes
>  E#E_MAX ( 2 ) + 3
>  s#bar struct bar { int mem [ E#E_MAX ] ; }
>  foo int foo ( s#bar * )
>
> The __add_symbol() function includes logic to handle the incrementation of
> enumeration values, but this code is also invoked when reading a reference
> file. As a result, the variables last_enum_expr and enum_counter might be
> incorrectly set after reading the reference file, which later affects
> parsing of the actual input.
>
> Fix the problem by splitting the logic for the incrementation of
> enumeration values into a separate function process_enum() and call it from
> __add_symbol() only when processing non-reference data.
>
> Fixes: e37ddb825003 ("genksyms: Track changes to enum constants")
> Signed-off-by: Petr Pavlu <petr.pavlu@...e.com>
> ---
>
> Changes since v1 [1]:
> - Remove the unnecessary condition 'type == SYM_ENUM' in process_enum().


Applied to linux-kbuild. Thanks.



> [1] https://lore.kernel.org/linux-kbuild/20250527142318.14175-1-petr.pavlu@suse.com/
>
>  scripts/genksyms/genksyms.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c
> index 8b0d7ac73dbb..83e48670c2fc 100644
> --- a/scripts/genksyms/genksyms.c
> +++ b/scripts/genksyms/genksyms.c
> @@ -181,13 +181,9 @@ static int is_unknown_symbol(struct symbol *sym)
>                         strcmp(defn->string, "{") == 0);
>  }
>
> -static struct symbol *__add_symbol(const char *name, enum symbol_type type,
> -                           struct string_list *defn, int is_extern,
> -                           int is_reference)
> +static struct string_list *process_enum(const char *name, enum symbol_type type,
> +                                       struct string_list *defn)
>  {
> -       unsigned long h;
> -       struct symbol *sym;
> -       enum symbol_status status = STATUS_UNCHANGED;
>         /* The parser adds symbols in the order their declaration completes,
>          * so it is safe to store the value of the previous enum constant in
>          * a static variable.
> @@ -216,7 +212,7 @@ static struct symbol *__add_symbol(const char *name, enum symbol_type type,
>                                 defn = mk_node(buf);
>                         }
>                 }
> -       } else if (type == SYM_ENUM) {
> +       } else {
>                 free_list(last_enum_expr, NULL);
>                 last_enum_expr = NULL;
>                 enum_counter = 0;
> @@ -225,6 +221,23 @@ static struct symbol *__add_symbol(const char *name, enum symbol_type type,
>                         return NULL;
>         }
>
> +       return defn;
> +}
> +
> +static struct symbol *__add_symbol(const char *name, enum symbol_type type,
> +                           struct string_list *defn, int is_extern,
> +                           int is_reference)
> +{
> +       unsigned long h;
> +       struct symbol *sym;
> +       enum symbol_status status = STATUS_UNCHANGED;
> +
> +       if ((type == SYM_ENUM_CONST || type == SYM_ENUM) && !is_reference) {
> +               defn = process_enum(name, type, defn);
> +               if (defn == NULL)
> +                       return NULL;
> +       }
> +
>         h = crc32(name);
>         hash_for_each_possible(symbol_hashtable, sym, hnode, h) {
>                 if (map_to_ns(sym->type) != map_to_ns(type) ||
>
> base-commit: 546b1c9e93c2bb8cf5ed24e0be1c86bb089b3253
> --
> 2.49.0
>
>


-- 
Best Regards
Masahiro Yamada