mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2025-12-27 11:06:41 -05:00
genksyms: fix memory leak when the same symbol is added from source
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 5dae9a550a
("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/?id=46bd1da672d66ccd8a639d3c1f8a166048cca608
Fixes: 5dae9a550a ("genksyms: allow to ignore symbol checksum changes")
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
This commit is contained in:
@@ -239,6 +239,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) {
|
||||
@@ -247,6 +248,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) ?
|
||||
@@ -254,6 +256,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;
|
||||
|
||||
@@ -152,14 +152,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;
|
||||
@@ -170,6 +175,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;
|
||||
|
||||
Reference in New Issue
Block a user