Hi,
On Wed, Jan 08, 2025 at 01:34:24PM +0900, Akihiko Odaki wrote:
> On 2025/01/08 1:18, Dave Martin wrote:
> > On Tue, Jan 07, 2025 at 09:45:53PM +0900, Akihiko Odaki wrote:
> > > Use note name macros to match with the userspace's expectation.
> >
> > Also (and more importantly) get rid of duplicated knowledge about the
> > mapping of note types to note names, so that elf.h is the authoritative
> > source of this information?
> >
> > >
> > > Signed-off-by: Akihiko Odaki <akihiko.odaki@xxxxxxxxxx>
> > > Acked-by: Baoquan He <bhe@xxxxxxxxxx>
> > > ---
> > > fs/binfmt_elf.c | 21 ++++++++++-----------
> > > fs/binfmt_elf_fdpic.c | 8 ++++----
> > > 2 files changed, 14 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> > > index 106f0e8af177..5b4a92e5e508 100644
> > > --- a/fs/binfmt_elf.c
> > > +++ b/fs/binfmt_elf.c
> >
> > [...]
> >
> > > @@ -1538,7 +1538,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
> > > do
> > > i += 2;
> > > while (auxv[i - 2] != AT_NULL);
> > > - fill_note(&auxv_note, "CORE", NT_AUXV, i * sizeof(elf_addr_t), auxv);
> > > + fill_note(&auxv_note, NN_AUXV, NT_AUXV, i * sizeof(elf_addr_t), auxv);
> > > thread_status_size += notesize(&auxv_note);
> > > offset = sizeof(*elf); /* ELF header */
> >
> > Looking at this code, it appears that the right name is explicitly
> > taken from elf.h for a few specific notes, but for those that are
> > specified by the arch code (e.g., in struct user_regset entries) the
> > name is still guessed locally:
> >
> > static int fill_thread_core_info(...) {
> >
> > ...
> >
> > fill_note(&t->notes[note_iter], is_fpreg ? "CORE" : "LINUX",
> > note_type, ret, data);
> >
> >
> > It would be preferable to clean this up if we want elf.h to be the
> > authoritative source for the names.
>
> If we want elf.h to be the authoritative source, yes, but I like the current
> form as it ensures nobody adds a note with a name different from "LINUX" and
> it is also simpler. There is a trade-off so I'd like to keep the current
> form unless anyone has a strong preference for one option.
>
> Regards,
> Akihiko Odaki
I can see where you're coming from here.
It would be nice to at least be able to check that elf.h is consistent
with the behaviour here, but you're right -- there is a tradeoff.
Maybe add a comment in elf.h at the end of the block of #defines saying
that new Linux-specific entries should use the name "LINUX"?
Either way, I don't think it's a huge deal. If people are happy with
this code as-is, then I don't have an issue with it.
I might follow up with a separate patch if this series is merged, and
people can consider it on its own merits (or lack thereof).
Cheers
---Dave
