Hi Anton,

On Thu, Feb 13, 2025 at 07:33:12PM +0300, Anton Moryakov wrote:
> Static analyzer reported:
> Return value of a function 'elf_strptr' is dereferenced at readelf.c:7171
> without checking for NULL, but it is usually checked for this function 
> (71/74).
> 
> Corrections explained:
> - Added a NULL check for the scnname variable, which contains the result of
>   the elf_strptr call.
> - The check is placed before the first use of scnname to prevent dereferencing
>   a NULL pointer.

Yes, I see why a static analyzer might assume that.  But if you look
at the caller of print_debug_frame_section, the function print_debug,
then you'll see that it already does this check (at line 12140):

          const char *name = elf_strptr (ebl->elf, shstrndx,
                                         shdr->sh_name); 
          if (name == NULL)
            continue;

And at the start of print_debug_frame_section we do have:

  /* We know this call will succeed since it did in the caller.  */
  (void) elf_getshdrstrndx (ebl->elf, &shstrndx);
  const char *scnname = elf_strptr (ebl->elf, shstrndx, shdr->sh_name);

So I don't think scnname can be NULL.

But this code is different from any other print_debug_* code.  All
other code uses section_name (Ebl *ebl, GElf_Shdr *shdr) to get the
section name.

So what we could do to make the static analyzer happy is simply do the
same here. I pushed the attached.

Cheers,

Mark
>From dfa7b2c23ddabcba2a4972fa67d3c670ae31f1ee Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Thu, 27 Feb 2025 21:22:49 +0100
Subject: [PATCH] readelf: Use section_name instead of elf_strptr in
 print_debug_frame_section

All other print_debug_* functions use section_name (ebl, shdr) to get
the current section name. Be consistent and use the same method in
print_debug_frame_section to make static analyzers happy who might
think elf_strptr can return NULL in this case.

      * src/readelf.c (print_debug_frame_section): Use section_name
      instead of elf_strptr to get the section name.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 src/readelf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/readelf.c b/src/readelf.c
index 61d5b71a7913..c9aebd5b3e5f 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -7170,7 +7170,7 @@ print_debug_frame_section (Dwfl_Module *dwflmod, Ebl 
*ebl, GElf_Ehdr *ehdr,
   size_t shstrndx;
   /* We know this call will succeed since it did in the caller.  */
   (void) elf_getshdrstrndx (ebl->elf, &shstrndx);
-  const char *scnname = elf_strptr (ebl->elf, shstrndx, shdr->sh_name);
+  const char *scnname = section_name (ebl, shdr);
 
   /* Needed if we find PC-relative addresses.  */
   GElf_Addr bias;
-- 
2.48.1

Reply via email to