Hi Mark,

On Tue, Oct 21, 2025 at 10:29 AM Mark Wielaard <[email protected]> wrote:
>
> Hi Aaron,
>
> On Thu, 2025-10-09 at 17:08 -0400, Aaron Merey wrote:
> > check_note_data compares the type of a given ELF note to a list of known
> > types.  If the type is not recognized then an "unknown note" error is
> > raised.
> >
> > Unknown note types do not necessarily indicate a gABI/psABI compliance
> > issue so an error should not be raised for this reason alone.
>
> Yes, but I did find it useful to see if there were any note types we
> didn't handle. I am not against this cleanup, but wouldn't mind a mode
> where it did warn about unknown note types. Maybe behind if (be_strict)
> --strict flag? Could be a new feature if you don't feel like doing it
> in this patch, in that case, could you file a bug for it?

Thanks, I've pushed this patch and filed bugs for ET_CORE note sanity
checks as well as --strict raising "unknown note type" errors:
https://sourceware.org/bugzilla/show_bug.cgi?id=33568
https://sourceware.org/bugzilla/show_bug.cgi?id=33567

Aaron

> > Additionally some common note types are missing from the list of known
> > types (NT_FILE for example).
>
> And NT_SIGINFO. Both FILE and SIGINFO are handled by eu-readelf -n
> though (including sanity checking). Both are CORE notes.
>
> > Fix this by removing the "unknown note" error from check_note_data.
> >
> > This patch preserves existing type-specific format checks and adds a
> > check for the presence of the null terminator in the note name, if
> > applicable.  If one of these checks fails, a "malformed note" or
> > "missing null terminator" error is raised.
>
> Note that in backends/linux-core-note.c there is an exception for not
> zero terminated "CORE" or "LINUX" notes /* Buggy old Linux kernels.  */
>
> But these are probably so old now that they don't occur anymore.
>
> > Since there are currently no type-specific checks for any ET_CORE
> > notes, these checks are only performed when the given binary does not
> > have e_type ET_CORE.
>
> Could we file a bug for this. It would be useful if some sanity checks
> were also done for core notes (like eu-readelf already does for some).
>
> > https://sourceware.org/bugzilla/show_bug.cgi?id=33488
> >
> > Signed-off-by: Aaron Merey <[email protected]>
> > ---
> >  src/elflint.c | 106 +++++++++++++++++++++++++++-----------------------
> >  1 file changed, 58 insertions(+), 48 deletions(-)
> >
> > diff --git a/src/elflint.c b/src/elflint.c
> > index a46682c7..daa9410d 100644
> > --- a/src/elflint.c
> > +++ b/src/elflint.c
> > @@ -4368,41 +4368,47 @@ check_note_data (Ebl *ebl, const GElf_Ehdr *ehdr,
> >      {
> >        last_offset = offset;
> >
> > -      /* Make sure it is one of the note types we know about.  */
> > -      if (ehdr->e_type == ET_CORE)
> > -     switch (nhdr.n_type)
> > -       {
> > -       case NT_PRSTATUS:
> > -       case NT_FPREGSET:
> > -       case NT_PRPSINFO:
> > -       case NT_TASKSTRUCT:           /* NT_PRXREG on Solaris.  */
> > -       case NT_PLATFORM:
> > -       case NT_AUXV:
> > -       case NT_GWINDOWS:
> > -       case NT_ASRS:
> > -       case NT_PSTATUS:
> > -       case NT_PSINFO:
> > -       case NT_PRCRED:
> > -       case NT_UTSNAME:
> > -       case NT_LWPSTATUS:
> > -       case NT_LWPSINFO:
> > -       case NT_PRFPXREG:
> > -         /* Known type.  */
> > -         break;
> > +      /* gelf_getnote verified that this note is aligned and does not 
> > extend
> > +      outside of DATA.  Now check that the note name is null-terminated
> > +      if present.  */
> > +      if (name_offset != 0
> > +       && nhdr.n_namesz > 0
> > +       && *((char *) data->d_buf + name_offset + nhdr.n_namesz - 1) != 
> > '\0')
> > +     {
> > +       if (ehdr->e_type == ET_CORE)
> > +         {
> > +           if (shndx == 0)
> > +             ERROR (_("\
> > +phdr[%d]: name missing null terminator for core file note with type %" 
> > PRIu32
> > +                         " at offset %" PRIu64 "\n"),
> > +                    phndx, (uint32_t) nhdr.n_type, start + offset);
> > +           else
> > +             ERROR (_("\
> > +section [%2d] '%s': name missing null terminator for core file note with "
> > +                         "type %" PRIu32 " at offset %zu\n"),
> > +                    shndx, section_name (ebl, shndx),
> > +                    (uint32_t) nhdr.n_type, offset);
> > +         }
> > +       else
> > +         {
> > +           if (shndx == 0)
> > +             ERROR (_("\
> > +phdr[%d]: name missing null terminator for object file note with type %" 
> > PRIu32
> > +                         " at offset %zu\n"),
> > +                    phndx, (uint32_t) nhdr.n_type, offset);
> > +           else
> > +             ERROR (_("\
> > +section [%2d] '%s': name missing null terminator for object file note with 
> > "
> > +                         "type %" PRIu32 " at offset %zu\n"),
> > +                    shndx, section_name (ebl, shndx),
> > +                    (uint32_t) nhdr.n_type, offset);
> > +         }
>
> OK.
>
> > -       default:
> > -         if (shndx == 0)
> > -           ERROR (_("\
> > -phdr[%d]: unknown core file note type %" PRIu32 " at offset %" PRIu64 
> > "\n"),
> > -                  phndx, (uint32_t) nhdr.n_type, start + offset);
> > -         else
> > -           ERROR (_("\
> > -section [%2d] '%s': unknown core file note type %" PRIu32
> > -                           " at offset %zu\n"),
> > -                  shndx, section_name (ebl, shndx),
> > -                  (uint32_t) nhdr.n_type, offset);
> > -       }
> > -      else
> > +       continue;
> > +     }
> > +
> > +      /* Perform type-specific checks.  */
> > +      if (ehdr->e_type != ET_CORE)
> >       switch (nhdr.n_type)
> >         {
> >         case NT_GNU_ABI_TAG:
> > @@ -4412,15 +4418,15 @@ section [%2d] '%s': unknown core file note type %" 
> > PRIu32
> >         case NT_GNU_PROPERTY_TYPE_0:
> >           if (nhdr.n_namesz == sizeof ELF_NOTE_GNU
> >               && strcmp (data->d_buf + name_offset, ELF_NOTE_GNU) == 0)
> > -           break;
> > +           continue;
> >           else
> >             {
> >               /* NT_VERSION is 1, same as NT_GNU_ABI_TAG.  It has no
> >                  descriptor and (ab)uses the name as version string.  */
> >               if (nhdr.n_descsz == 0 && nhdr.n_type == NT_VERSION)
> > -               break;
> > +               continue;
> >             }
> > -           goto unknown_note;
> > +           goto malformed_note;
> >
> >         case NT_GNU_BUILD_ATTRIBUTE_OPEN:
> >         case NT_GNU_BUILD_ATTRIBUTE_FUNC:
> > @@ -4431,39 +4437,43 @@ section [%2d] '%s': unknown core file note type %" 
> > PRIu32
> >               && strncmp (data->d_buf + name_offset,
> >                           ELF_NOTE_GNU_BUILD_ATTRIBUTE_PREFIX,
> >                           strlen (ELF_NOTE_GNU_BUILD_ATTRIBUTE_PREFIX)) == 
> > 0)
> > -           break;
> > +           continue;
> >           else
> > -           goto unknown_note;
> > +           goto malformed_note;
> >
> >         case NT_FDO_PACKAGING_METADATA:
> >           if (nhdr.n_namesz == sizeof ELF_NOTE_FDO
> >               && strcmp (data->d_buf + name_offset, ELF_NOTE_FDO) == 0)
> > -           break;
> > +           continue;
> >           else
> > -           goto unknown_note;
> > +           goto malformed_note;
> >
> >         case 0:
> >           /* Linux vDSOs use a type 0 note for the kernel version word.  */
> >           if (nhdr.n_namesz == sizeof "Linux"
> >               && !memcmp (data->d_buf + name_offset, "Linux", sizeof 
> > "Linux"))
> > -           break;
> > -         FALLTHROUGH;
> > +           continue;
> > +         else
> > +           goto malformed_note;
> >         default:
> > -         {
> > -         unknown_note:
> > +         /* n_type not recognized, but no errors found regarding alignment,
> > +            overflow or name null terminator.  */
> > +         continue;
>
> Slightly hard to see through the if-else-continue-gotos, but looks
> correct.
>
> > +malformed_note:
> >           if (shndx == 0)
> >             ERROR (_("\
> > -phdr[%d]: unknown object file note type %" PRIu32 " with owner name '%s' 
> > at offset %zu\n"),
> > +phdr[%d]: malformed object file note type %" PRIu32 " with owner name '%s' 
> > "
> > +                           "at offset %zu\n"),
> >                    phndx, (uint32_t) nhdr.n_type,
> >                    (char *) data->d_buf + name_offset, offset);
> >           else
> >             ERROR (_("\
> > -section [%2d] '%s': unknown object file note type %" PRIu32
> > +section [%2d] '%s': malformed object file note type %" PRIu32
> >                             " with owner name '%s' at offset %zu\n"),
> >                    shndx, section_name (ebl, shndx),
> >                    (uint32_t) nhdr.n_type,
> >                    (char *) data->d_buf + name_offset, offset);
> > -         }
> >         }
> >      }
> >
>
> OK.
>
> Thanks,
>
> Mark
>

Reply via email to