Hi,
static analysis tools complain that the following code lacks a null-pointer
check:
ldso/ldso/dl-elf.c:
/*
* Add this object into the symbol chain
*/
if (*rpnt
#ifdef __LDSO_STANDALONE_SUPPORT__
/* Do not create a new chain entry for the main executable */
&& (*rpnt)->dyn
#endif
) {
(*rpnt)->next = _dl_malloc(sizeof(struct dyn_elf));
_dl_memset((*rpnt)->next, 0, sizeof(struct dyn_elf));
(*rpnt)->next->prev = (*rpnt);
*rpnt = (*rpnt)->next;
}
#ifndef SHARED
/* When statically linked, the first time we dlopen a DSO
* the *rpnt is NULL, so we need to allocate memory for it,
* and initialize the _dl_symbol_table.
*/
else {
*rpnt = _dl_symbol_tables = _dl_malloc(sizeof(struct dyn_elf));
_dl_memset(*rpnt, 0, sizeof(struct dyn_elf));
}
#endif
(*rpnt)->dyn = tpnt;
^^^^^^^^^^^^^^^^^^^^
There is a check for (*rpnt == NULL) right after the first comment but the
"else" case which performs an allocation does only exist if SHARED is not
defined. Otherwise it may happen (at least in theory) that *rpnt=NULL when
executing
(*rpnt)->dyn = tpnt;
Proposed fix:
diff --git a/ldso/ldso/dl-elf.c b/ldso/ldso/dl-elf.c
index 8210a012e..3ba3144e2 100644
--- a/ldso/ldso/dl-elf.c
+++ b/ldso/ldso/dl-elf.c
@@ -900,7 +900,8 @@ struct elf_resolve *_dl_load_elf_shared_library(unsigned int rflags,
_dl_memset(*rpnt, 0, sizeof(struct dyn_elf));
}
#endif
- (*rpnt)->dyn = tpnt;
+ if (*rpnt)
+ (*rpnt)->dyn = tpnt;
tpnt->usage_count++;
if (tpnt->rtld_flags & RTLD_NODELETE)
tpnt->usage_count++;
Kind regards
Frank
Hi,
I stumbled upon the following code in tempname.c:
switch (kind) {
case __GT_NOCREATE:
{
struct stat st;
if (stat (tmpl, &st) < 0) {
if (errno == ENOENT) {
fd = 0;
goto restore_and_ret;
} else
/* Give up now. */
return -1;
} else
fd = 0;
}
case __GT_FILE:
fd = open (tmpl, O_RDWR | O_CREAT | O_EXCL | flags,
mode);
break;
I am baffled by the 'else' branch of the check of 'stat (tmpl, &st) <
0'.
If I understand the whole thing correctly, this is an error case,
where
a file with the temporary name already exists. But instead, the whole
execution falls through to the __GT_FILE branch, and actually opens the
file.
If this happens, this file never seems to be closed again (see for
example
the usage in tmpnam_r.c:30:
char * tmpnam_r (char *s)
{
if (s == NULL)
return NULL;
if (__path_search (s, L_tmpnam, NULL, NULL, 0))
return NULL;
if (__gen_tempname (s, __GT_NOCREATE, 0, 0, 0))
return NULL;
return s;
}
I would suppose that instead of falling to the next switch-case, we
should return with a non-zero value (maybe '1' to distinguish it from
the general error case?).
Am I missing something basic?
Cheers,
Sven
--
Sven Linker
Tel.: +49 351 41883243
Kernkonzept GmbH at Dresden, Germany,
HRB 31129, CEO Dr.-Ing. Michael Hohmuth
Hi,
I fixed two kinds of Clang warnings in uclibc-ng (see patches attached).
Is that something you would be willing to include?
Best regards,
Marius
--
Marius Melzer, marius.melzer(a)kernkonzept.com
Kernkonzept GmbH, Dresden, Germany, HRB 31129, CEO Dr.-Ing. Michael Hohmuth
