diff mbox series

[uclibc-ng-devel] dl-elf.c: Add null-pointer check

Message ID 6013950.lOV4Wx5bFT@noys4
State Accepted
Headers show
Series [uclibc-ng-devel] dl-elf.c: Add null-pointer check | expand

Commit Message

Frank Mehnert Jan. 30, 2024, 12:14 p.m. UTC
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:




Kind regards

Frank

Comments

Waldemar Brodkorb Feb. 11, 2024, 6:11 a.m. UTC | #1
Hi Frank,

could you sent a patch including your good description with your
Signed-Off-By? For example with git format-patch -s origin

Thanks in advance
 Waldemar

Frank Mehnert wrote,

> 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
> 
> 
> _______________________________________________
> devel mailing list -- devel@uclibc-ng.org
> To unsubscribe send an email to devel-leave@uclibc-ng.org
>
diff mbox series

Patch

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++;