5.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Masahiro Yamada masahiroy@kernel.org
[ Upstream commit 45c9c4101d3d2fdfa00852274bbebba65fcc3cf2 ]
When a symbol that is already registered is added again, __add_symbol() returns without freeing the symbol definition, making it unreachable.
The following test cases demonstrate different memory leak points.
[Test Case 1]
Forward declaration with exactly the same definition
$ cat foo.c #include <linux/export.h> void foo(void); void foo(void) {} EXPORT_SYMBOL(foo);
[Test Case 2]
Forward declaration with a different definition (e.g. attribute)
$ cat foo.c #include <linux/export.h> void foo(void); __attribute__((__section__(".ref.text"))) void foo(void) {} EXPORT_SYMBOL(foo);
[Test Case 3]
Preserving an overridden symbol (compile with KBUILD_PRESERVE=1)
$ cat foo.c #include <linux/export.h> void foo(void); void foo(void) { } EXPORT_SYMBOL(foo);
$ cat foo.symref override foo void foo ( int )
The memory leaks in Test Case 1 and 2 have existed since the introduction of genksyms into the kernel tree. [1]
The memory leak in Test Case 3 was introduced by commit 5dae9a550a74 ("genksyms: allow to ignore symbol checksum changes").
When multiple init_declarators are reduced to an init_declarator_list, the decl_spec must be duplicated. Otherwise, the following Test Case 4 would result in a double-free bug.
[Test Case 4]
$ cat foo.c #include <linux/export.h>
extern int foo, bar;
int foo, bar; EXPORT_SYMBOL(foo);
In this case, 'foo' and 'bar' share the same decl_spec, 'int'. It must be unshared before being passed to add_symbol().
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?...
Fixes: 5dae9a550a74 ("genksyms: allow to ignore symbol checksum changes") Signed-off-by: Masahiro Yamada masahiroy@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- scripts/genksyms/genksyms.c | 3 +++ scripts/genksyms/parse.y | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c index 23eff234184f3..d74bad87ef1a7 100644 --- a/scripts/genksyms/genksyms.c +++ b/scripts/genksyms/genksyms.c @@ -241,6 +241,7 @@ static struct symbol *__add_symbol(const char *name, enum symbol_type type, "unchanged\n"); } sym->is_declared = 1; + free_list(defn, NULL); return sym; } else if (!sym->is_declared) { if (sym->is_override && flag_preserve) { @@ -249,6 +250,7 @@ static struct symbol *__add_symbol(const char *name, enum symbol_type type, print_type_name(type, name); fprintf(stderr, " modversion change\n"); sym->is_declared = 1; + free_list(defn, NULL); return sym; } else { status = is_unknown_symbol(sym) ? @@ -256,6 +258,7 @@ static struct symbol *__add_symbol(const char *name, enum symbol_type type, } } else { error_with_pos("redefinition of %s", name); + free_list(defn, NULL); return sym; } break; diff --git a/scripts/genksyms/parse.y b/scripts/genksyms/parse.y index e22b42245bcc2..7df3fe290d535 100644 --- a/scripts/genksyms/parse.y +++ b/scripts/genksyms/parse.y @@ -149,14 +149,19 @@ simple_declaration: ;
init_declarator_list_opt: - /* empty */ { $$ = NULL; } - | init_declarator_list + /* empty */ { $$ = NULL; } + | init_declarator_list { free_list(decl_spec, NULL); $$ = $1; } ;
init_declarator_list: init_declarator { struct string_list *decl = *$1; *$1 = NULL; + + /* avoid sharing among multiple init_declarators */ + if (decl_spec) + decl_spec = copy_list_range(decl_spec, NULL); + add_symbol(current_name, is_typedef ? SYM_TYPEDEF : SYM_NORMAL, decl, is_extern); current_name = NULL; @@ -167,6 +172,11 @@ init_declarator_list: *$3 = NULL; free_list(*$2, NULL); *$2 = decl_spec; + + /* avoid sharing among multiple init_declarators */ + if (decl_spec) + decl_spec = copy_list_range(decl_spec, NULL); + add_symbol(current_name, is_typedef ? SYM_TYPEDEF : SYM_NORMAL, decl, is_extern); current_name = NULL;