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? > 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
