Re: [PATCH v4 1/6] elf: Define note name macros

[Dave Martin <Dave.Martin@arm.com>]

Hi,

On Sat, Jan 11, 2025 at 02:48:44PM +0900, Akihiko Odaki wrote:
> elf.h had a comment saying:
> > Notes used in ET_CORE. Architectures export some of the arch register
> > sets using the corresponding note types via the PTRACE_GETREGSET and
> > PTRACE_SETREGSET requests.
> > The note name for these types is "LINUX", except NT_PRFPREG that is
> > named "CORE".
> 
> However, NT_PRSTATUS is also named "CORE". It is also unclear what
> "these types" refers to.
> 
> To fix these problems, define a name for each note type. The added
> definitions are macros so the kernel and userspace can directly refer to
> them.

I guess another motivation is to move towards having elf.h as the
single point of definition for the note names, so that obscure and
duplicate logic for determining note names can be removed.

(Could be worth adding a note on this is the patch is respun, but this
would be overkill otherwise...)


> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> Acked-by: Baoquan He <bhe@redhat.com>
> ---
>  include/uapi/linux/elf.h | 86 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 83 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
> index b44069d29cec..343f5c40d03a 100644
> --- a/include/uapi/linux/elf.h
> +++ b/include/uapi/linux/elf.h
> @@ -368,99 +368,179 @@ typedef struct elf64_shdr {
>  #define ELF_OSABI ELFOSABI_NONE
>  #endif
>  
> +/* Note definitions: NN_ defines names. NT_ defines types. */
> +
>  /*
>   * Notes used in ET_CORE. Architectures export some of the arch register sets
>   * using the corresponding note types via the PTRACE_GETREGSET and
>   * PTRACE_SETREGSET requests.
> - * The note name for these types is "LINUX", except NT_PRFPREG that is named
> - * "CORE".
>   */
> +#define NN_PRSTATUS	"CORE"
>  #define NT_PRSTATUS	1
> +#define NN_PRFPREG	"CORE"
>  #define NT_PRFPREG	2
> +#define NN_PRPSINFO	"CORE"
>  #define NT_PRPSINFO	3
> +#define NN_TASKSTRUCT	"CORE"
>  #define NT_TASKSTRUCT	4
> +#define NN_AUXV		"CORE"
>  #define NT_AUXV		6
>  /*
>   * Note to userspace developers: size of NT_SIGINFO note may increase
>   * in the future to accomodate more fields, don't assume it is fixed!
>   */
> +#define NN_SIGINFO      "CORE"
>  #define NT_SIGINFO      0x53494749
> +#define NN_FILE         "CORE"
>  #define NT_FILE         0x46494c45
> +#define NN_PRXFPREG     "LINUX"
>  #define NT_PRXFPREG     0x46e62b7f      /* copied from gdb5.1/include/elf/common.h */

[...]

> +#define NN_LOONGARCH_HW_WATCH	"LINUX"
>  #define NT_LOONGARCH_HW_WATCH	0xa06   /* LoongArch hardware watchpoint registers */
>  
> -/* Note types with note name "GNU" */
> +/* Note used in other file types. */

For this, it may make sense to be vaguer, e.g.

	/* Other notes */

(We don't dump NT_GNU_PROPERTY_TYPE_0 in coredumps, but I don't think
it would be nonsensical to do so.)

> +#define NN_GNU_PROPERTY_TYPE_0	"GNU"
>  #define NT_GNU_PROPERTY_TYPE_0	5

[...]

With or without those changes,

Reviewed-by: Dave Martin <Dave.Martin@arm.com>