Subject: Fix segmentation violation in symbol_search
Fix a possible segmentation violation in crash if a module name
is not NUL-terminated. Although store_module_symbols_v2 complains
about an overly long module name, there are several problems
with the current approach:
1. The maximum size is hard-wired in defs.h and the current
constant doesn't even match struct module's name field size
on any architecture.
2. If the string is too long, it is probably not NUL-terminated,
so we can't use strlen() on it.
3. Even though only the first MAX_MOD_NAME-1 bytes are copied
to struct load_module, the _MODULE_* pseudo-symbol names are
generated from the unabridged module name. As a consequence,
they are not found further on in the loop at the end of
store_module_symbols_v2, so lm->mod_symtable remains NULL
for that module. The symbol_search() function is not
prepared for that situation and tries to dereference that
NULL pointer here:
sp = lm->mod_symtable;
sp_end = lm->mod_symend;
for ( ; sp <= sp_end; sp++) {
if (!pseudos && MODULE_PSEUDO_SYMBOL(sp))
^^^^
Regards,
Petr Tesarik
Subject: Fix segmentation violation in symbol_search
Fix a possible segmentation violation in crash if a module name
is not NUL-terminated. Although store_module_symbols_v2 complains
about an overly long module name, there are several problems
with the current approach:
1. The maximum size is hard-wired in defs.h and the current
constant doesn't even match struct module's name field size
on any architecture.
2. If the string is too long, it is probably not NUL-terminated,
so we can't use strlen() on it.
3. Even though only the first MAX_MOD_NAME-1 bytes are copied
to struct load_module, the _MODULE_* pseudo-symbol names are
generated from the unabridged module name. As a consequence,
they are not found further on in the loop at the end of
store_module_symbols_v2, so lm->mod_symtable remains NULL
for that module. The symbol_search() function is not
prepared for that situation and tries to dereference that
NULL pointer here:
sp = lm->mod_symtable;
sp_end = lm->mod_symend;
for ( ; sp <= sp_end; sp++) {
if (!pseudos && MODULE_PSEUDO_SYMBOL(sp))
^^^^
Signed-off-by: Petr Tesarik <ptesarik@xxxxxxx>
---
symbols.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
--- a/symbols.c
+++ b/symbols.c
@@ -1337,6 +1337,7 @@ store_module_symbols_v2(ulong total, int
char *strbuf, *modbuf, *modsymbuf;
struct syment *sp;
ulong first, last;
+ size_t max_mod_name;
st->mods_installed = mods_installed;
@@ -1367,6 +1368,16 @@ store_module_symbols_v2(ulong total, int
(st->mods_installed, sizeof(struct load_module))) == NULL)
error(FATAL, "load_module array malloc: %s\n", strerror(errno));
+ max_mod_name = MEMBER_SIZE("module", "name");
+ if (max_mod_name > MAX_MOD_NAME) {
+ error(INFO, "size of module.name is larger than MAX_MOD_NAME"
+ " (%d > %d)", max_mod_name, MAX_MOD_NAME);
+ max_mod_name = MAX_MOD_NAME;
+ } else if (max_mod_name <= 0) {
+ error(INFO, "cannot determine max module name length");
+ max_mod_name = MAX_MOD_NAME;
+ }
+
modbuf = GETBUF(SIZE(module));
modsymbuf = NULL;
m = mcnt = mod_next = 0;
@@ -1396,13 +1407,14 @@ store_module_symbols_v2(ulong total, int
lm->mod_base = ULONG(modbuf + OFFSET(module_module_core));
lm->module_struct = mod;
lm->mod_size = size;
- if (strlen(mod_name) < MAX_MOD_NAME)
+ if (memchr(mod_name, 0, max_mod_name))
strcpy(lm->mod_name, mod_name);
else {
- error(INFO,
- "module name greater than MAX_MOD_NAME: %s\n",
- mod_name);
- strncpy(lm->mod_name, mod_name, MAX_MOD_NAME-1);
+ memcpy(lm->mod_name, mod_name, max_mod_name-1);
+ lm->mod_name[max_mod_name-1] = '\0';
+ error(INFO,
+ "module name too long: %s (truncated)\n",
+ lm->mod_name);
}
if (CRASHDEBUG(3))
fprintf(fp,
@@ -1434,7 +1446,7 @@ store_module_symbols_v2(ulong total, int
st->ext_module_symtable[mcnt].value = lm->mod_base;
st->ext_module_symtable[mcnt].type = 'm';
st->ext_module_symtable[mcnt].flags |= MODULE_SYMBOL;
- sprintf(buf2, "%s%s", "_MODULE_START_", mod_name);
+ sprintf(buf2, "%s%s", "_MODULE_START_", lm->mod_name);
namespace_ctl(NAMESPACE_INSTALL, &st->ext_module_namespace,
&st->ext_module_symtable[mcnt], buf2);
lm_mcnt = mcnt;
@@ -1444,7 +1456,7 @@ store_module_symbols_v2(ulong total, int
st->ext_module_symtable[mcnt].value = lm->mod_init_module_ptr;
st->ext_module_symtable[mcnt].type = 'm';
st->ext_module_symtable[mcnt].flags |= MODULE_SYMBOL;
- sprintf(buf3, "%s%s", "_MODULE_INIT_START_", mod_name);
+ sprintf(buf3, "%s%s", "_MODULE_INIT_START_", lm->mod_name);
namespace_ctl(NAMESPACE_INSTALL,
&st->ext_module_namespace,
&st->ext_module_symtable[mcnt], buf3);
@@ -1627,7 +1639,7 @@ store_module_symbols_v2(ulong total, int
st->ext_module_symtable[mcnt].value = lm->mod_base + size;
st->ext_module_symtable[mcnt].type = 'm';
st->ext_module_symtable[mcnt].flags |= MODULE_SYMBOL;
- sprintf(buf2, "%s%s", "_MODULE_END_", mod_name);
+ sprintf(buf2, "%s%s", "_MODULE_END_", lm->mod_name);
namespace_ctl(NAMESPACE_INSTALL,
&st->ext_module_namespace,
&st->ext_module_symtable[mcnt], buf2);
@@ -1637,7 +1649,7 @@ store_module_symbols_v2(ulong total, int
st->ext_module_symtable[mcnt].value = lm->mod_init_module_ptr + lm->mod_init_size;
st->ext_module_symtable[mcnt].type = 'm';
st->ext_module_symtable[mcnt].flags |= MODULE_SYMBOL;
- sprintf(buf4, "%s%s", "_MODULE_INIT_END_", mod_name);
+ sprintf(buf4, "%s%s", "_MODULE_INIT_END_", lm->mod_name);
namespace_ctl(NAMESPACE_INSTALL,
&st->ext_module_namespace,
&st->ext_module_symtable[mcnt], buf4);
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
