[RFC PATCH 0/2] ELF: Alternate program property parser

[RFC PATCH 0/2] ELF: Alternate program property parser

[Dave Martin]

This series is an experimental reimplementation of ELF property parsing
(see NT_GNU_PROPERTY_TYPE_0, [1]) for the ELF loader.

This is intended for comparison / merging with [2] (or could replace it,
if people think this approach is better).

Either way, I'd like to get something in place so that we can build
AArch64 BTI support on top of it.

Any thoughts?


Key differences from [2]:

 * Scanning for the PT_PROGRAM_PROPERTY program header is intergrated
   into the existing scan loops, rather than being done separately.

 * In keeping with the rest of the ELF loader code, error checks are
   kept to a minimum.  Except to avoid buffer overruns, the ELF file is
   not checked for well-formedness.

   As a sanity check, the code still checks for a correct
   NT_GNU_PROPERTY_TYPE_0 note header at the start of the
   PT_PROGRAM_PROPERTY segment, but perhaps this isn't needed either.

 * 1K is statically allocated on the stack for the properties, and if
   the ELF properties are larger than that, the ELF file is rejected
   with ENOEXEC.

   There is no limit defined in [1] for the total size of the
   properties, but common sense seems suggests that 1K is likely to be
   ample space.

 * The properties are found, read and parsed exactly once.  [2] does
   this once _per property_ requested by the arch code: that's not a
   problem today, but it will become inefficient with there are multiple
   properties in the file that the kernel needs to look at.

   Instead, the arch arch_parse_elf_property() hook is called once per
   property found.  To minimise overhead, the arch code can implement
   this hook inline.

   This approach assumes that the number of properties in a given ELF is
   say, no more than 20 or so.  The code could be redesigned in the
   future if/when this iteration becomes an overhead (i.e., probably
   never).


[1] Linux Extensions to gABI
https://github.com/hjl-tools/linux-abi/wiki/Linux-Extensions-to-gABI

[2] [PATCH v8 22/27] binfmt_elf: Extract .note.gnu.property from an ELF file
https://lore.kernel.org/lkml/20190813205225.12032-23-yu-cheng.yu@intel.com/


Dave Martin (2):
  ELF: UAPI and Kconfig additions for ELF program properties
  ELF: Add ELF program property parsing support

 fs/Kconfig.binfmt        |   3 ++
 fs/binfmt_elf.c          | 109 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/compat_binfmt_elf.c   |   4 ++
 include/linux/elf.h      |  21 +++++++++
 include/uapi/linux/elf.h |  11 +++++
 5 files changed, 148 insertions(+)

-- 
2.1.4

[RFC PATCH 1/2] ELF: UAPI and Kconfig additions for ELF program properties

[Dave Martin]

Pull the basic ELF definitions from Yu-Cheng Yu's series.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Dave Martin <Dave.Martin@arm.com>

---

This patch should be merged with the next patch.

I kept it seprate for now to document where this code came from.
---
 fs/Kconfig.binfmt        | 3 +++
 include/uapi/linux/elf.h | 7 +++++++
 2 files changed, 10 insertions(+)

diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt
index 62dc4f5..d2cfe07 100644
--- a/fs/Kconfig.binfmt
+++ b/fs/Kconfig.binfmt
@@ -36,6 +36,9 @@ config COMPAT_BINFMT_ELF
 config ARCH_BINFMT_ELF_STATE
 	bool
 
+config ARCH_USE_GNU_PROPERTY
+	bool
+
 config BINFMT_ELF_FDPIC
 	bool "Kernel support for FDPIC ELF binaries"
 	default y if !BINFMT_ELF
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index 34c02e4..a3eaad9 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -36,6 +36,7 @@ typedef __s64	Elf64_Sxword;
 #define PT_LOPROC  0x70000000
 #define PT_HIPROC  0x7fffffff
 #define PT_GNU_EH_FRAME		0x6474e550
+#define PT_GNU_PROPERTY		0x6474e553
 
 #define PT_GNU_STACK	(PT_LOOS + 0x474e551)
 
@@ -443,4 +444,10 @@ typedef struct elf64_note {
   Elf64_Word n_type;	/* Content type */
 } Elf64_Nhdr;
 
+/* NT_GNU_PROPERTY_TYPE_0 header */
+struct gnu_property {
+  __u32 pr_type;
+  __u32 pr_datasz;
+};
+
 #endif /* _UAPI_LINUX_ELF_H */
-- 
2.1.4

[RFC PATCH 2/2] ELF: Add ELF program property parsing support

[Dave Martin]

ELF program properties will needed for detecting whether to enable
optional architecture or ABI features for a new ELF process.

For now, there are no generic properties that we care about, so do
nothing unless CONFIG_ARCH_USE_GNU_PROPERTY=y.

Otherwise, the presence of properties using the PT_PROGRAM_PROPERTY
phdrs entry (if any), and notify each property to the arch code.

For now, the added code is not used.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 fs/binfmt_elf.c          | 109 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/compat_binfmt_elf.c   |   4 ++
 include/linux/elf.h      |  21 +++++++++
 include/uapi/linux/elf.h |   4 ++
 4 files changed, 138 insertions(+)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index d4e11b2..52f4b96 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -39,12 +39,18 @@
 #include <linux/sched/coredump.h>
 #include <linux/sched/task_stack.h>
 #include <linux/sched/cputime.h>
+#include <linux/sizes.h>
+#include <linux/types.h>
 #include <linux/cred.h>
 #include <linux/dax.h>
 #include <linux/uaccess.h>
 #include <asm/param.h>
 #include <asm/page.h>
 
+#ifndef ELF_COMPAT
+#define ELF_COMPAT 0
+#endif
+
 #ifndef user_long_t
 #define user_long_t long
 #endif
@@ -690,6 +696,93 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
 #endif
 }
 
+static int parse_elf_property(const void **prop, size_t *notesz,
+			      struct arch_elf_state *arch)
+{
+	const struct gnu_property *pr = *prop;
+	size_t sz = *notesz;
+	int ret;
+	size_t step;
+
+	BUG_ON(sz < sizeof(*pr));
+
+	if (sizeof(*pr) > sz)
+		return -EIO;
+
+	if (pr->pr_datasz > sz - sizeof(*pr))
+		return -EIO;
+
+	step = round_up(sizeof(*pr) + pr->pr_datasz, elf_gnu_property_align);
+	if (step > sz)
+		return -EIO;
+
+	ret = arch_parse_elf_property(pr->pr_type, *prop + sizeof(*pr),
+				      pr->pr_datasz, ELF_COMPAT, arch);
+	if (ret)
+		return ret;
+
+	*prop += step;
+	*notesz -= step;
+	return 0;
+}
+
+#define NOTE_DATA_SZ SZ_1K
+#define GNU_PROPERTY_TYPE_0_NAME "GNU"
+#define NOTE_NAME_SZ (sizeof(GNU_PROPERTY_TYPE_0_NAME))
+
+static int parse_elf_properties(struct file *f, const struct elf_phdr *phdr,
+				struct arch_elf_state *arch)
+{
+	ssize_t n;
+	loff_t pos = phdr->p_offset;
+	union {
+		struct elf_note nhdr;
+		char data[NOTE_DATA_SZ];
+	} note;
+	size_t off, notesz;
+	const void *prop;
+	int ret;
+
+	if (!IS_ENABLED(ARCH_USE_GNU_PROPERTY))
+		return 0;
+
+	BUG_ON(phdr->p_type != PT_GNU_PROPERTY);
+
+	/* If the properties are crazy large, that's too bad (for now): */
+	if (phdr->p_filesz > sizeof(note))
+		return -ENOEXEC;
+	n = kernel_read(f, &note, phdr->p_filesz, &pos);
+
+	BUILD_BUG_ON(sizeof(note) < sizeof(note.nhdr) + NOTE_NAME_SZ);
+	if (n < 0 || n < sizeof(note.nhdr) + NOTE_NAME_SZ)
+		return -EIO;
+
+	if (note.nhdr.n_type != NT_GNU_PROPERTY_TYPE_0 ||
+	    note.nhdr.n_namesz != NOTE_NAME_SZ ||
+	    strncmp(note.data + sizeof(note.nhdr),
+		    GNU_PROPERTY_TYPE_0_NAME, n - sizeof(note.nhdr)))
+		return -EIO;
+
+	off = round_up(sizeof(note.nhdr) + NOTE_NAME_SZ,
+		       elf_gnu_property_align);
+	if (off > n)
+		return -EIO;
+
+	prop = (const struct gnu_property *)(note.data + off);
+	notesz = n - off;
+	if (note.nhdr.n_descsz > notesz)
+		return -EIO;
+
+	while (notesz) {
+		BUG_ON(((char *)prop - note.data) % elf_gnu_property_align);
+		ret = parse_elf_property(&prop, &notesz, arch);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int load_elf_binary(struct linux_binprm *bprm)
 {
 	struct file *interpreter = NULL; /* to shut gcc up */
@@ -697,6 +790,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
 	int load_addr_set = 0;
 	unsigned long error;
 	struct elf_phdr *elf_ppnt, *elf_phdata, *interp_elf_phdata = NULL;
+	struct elf_phdr *elf_property_phdata = NULL;
 	unsigned long elf_bss, elf_brk;
 	int bss_prot = 0;
 	int retval, i;
@@ -744,6 +838,11 @@ static int load_elf_binary(struct linux_binprm *bprm)
 		char *elf_interpreter;
 		loff_t pos;
 
+		if (elf_ppnt->p_type == PT_GNU_PROPERTY) {
+			elf_property_phdata = elf_ppnt;
+			continue;
+		}
+
 		if (elf_ppnt->p_type != PT_INTERP)
 			continue;
 
@@ -839,9 +938,14 @@ out_free_interp:
 			goto out_free_dentry;
 
 		/* Pass PT_LOPROC..PT_HIPROC headers to arch code */
+		elf_property_phdata = NULL;
 		elf_ppnt = interp_elf_phdata;
 		for (i = 0; i < loc->interp_elf_ex.e_phnum; i++, elf_ppnt++)
 			switch (elf_ppnt->p_type) {
+			case PT_GNU_PROPERTY:
+				elf_property_phdata = elf_ppnt;
+				break;
+
 			case PT_LOPROC ... PT_HIPROC:
 				retval = arch_elf_pt_proc(&loc->interp_elf_ex,
 							  elf_ppnt, interpreter,
@@ -852,6 +956,11 @@ out_free_interp:
 			}
 	}
 
+	retval = parse_elf_properties(interpreter ?: bprm->file,
+				      elf_property_phdata, &arch_state);
+	if (retval)
+		goto out_free_dentry;
+
 	/*
 	 * Allow arch code to reject the ELF at this point, whilst it's
 	 * still possible to return an error to the code that invoked
diff --git a/fs/compat_binfmt_elf.c b/fs/compat_binfmt_elf.c
index b7f9ffa..f9ee476 100644
--- a/fs/compat_binfmt_elf.c
+++ b/fs/compat_binfmt_elf.c
@@ -17,6 +17,8 @@
 #include <linux/elfcore-compat.h>
 #include <linux/time.h>
 
+#define ELF_COMPAT	1
+
 /*
  * Rename the basic ELF layout types to refer to the 32-bit class of files.
  */
@@ -28,11 +30,13 @@
 #undef	elf_shdr
 #undef	elf_note
 #undef	elf_addr_t
+#undef	elf_gnu_property_align
 #define elfhdr		elf32_hdr
 #define elf_phdr	elf32_phdr
 #define elf_shdr	elf32_shdr
 #define elf_note	elf32_note
 #define elf_addr_t	Elf32_Addr
+#define elf_gnu_property_align	elf32_gnu_property_align
 
 /*
  * Some data types as stored in coredump.
diff --git a/include/linux/elf.h b/include/linux/elf.h
index e3649b3..886ffec 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -2,6 +2,7 @@
 #ifndef _LINUX_ELF_H
 #define _LINUX_ELF_H
 
+#include <linux/types.h>
 #include <asm/elf.h>
 #include <uapi/linux/elf.h>
 
@@ -21,6 +22,9 @@
 	SET_PERSONALITY(ex)
 #endif
 
+#define elf32_gnu_property_align	4
+#define elf64_gnu_property_align	8
+
 #if ELF_CLASS == ELFCLASS32
 
 extern Elf32_Dyn _DYNAMIC [];
@@ -31,6 +35,7 @@ extern Elf32_Dyn _DYNAMIC [];
 #define elf_addr_t	Elf32_Off
 #define Elf_Half	Elf32_Half
 #define Elf_Word	Elf32_Word
+#define elf_gnu_property_align	elf32_gnu_property_align
 
 #else
 
@@ -42,6 +47,7 @@ extern Elf64_Dyn _DYNAMIC [];
 #define elf_addr_t	Elf64_Off
 #define Elf_Half	Elf64_Half
 #define Elf_Word	Elf64_Word
+#define elf_gnu_property_align	elf64_gnu_property_align
 
 #endif
 
@@ -56,4 +62,19 @@ static inline int elf_coredump_extra_notes_write(struct coredump_params *cprm) {
 extern int elf_coredump_extra_notes_size(void);
 extern int elf_coredump_extra_notes_write(struct coredump_params *cprm);
 #endif
+
+struct arch_elf_state;
+
+#ifndef CONFIG_ARCH_USE_GNU_PROPERTY
+static inline int arch_parse_elf_property(u32 type, const void *data,
+					  size_t datasz, bool compat,
+					  struct arch_elf_state *arch)
+{
+	return 0;
+}
+#else
+extern int arch_parse_elf_property(u32 type, const void *data, size_t datasz,
+				   bool compat, struct arch_elf_state *arch);
+#endif
+
 #endif /* _LINUX_ELF_H */
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index a3eaad9..f03e3cf 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -368,6 +368,7 @@ typedef struct elf64_shdr {
  * 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 all these is "LINUX".
  */
 #define NT_PRSTATUS	1
 #define NT_PRFPREG	2
@@ -430,6 +431,9 @@ typedef struct elf64_shdr {
 #define NT_MIPS_FP_MODE	0x801		/* MIPS floating-point mode */
 #define NT_MIPS_MSA	0x802		/* MIPS SIMD registers */
 
+/* Note types with note name "GNU" */
+#define NT_GNU_PROPERTY_TYPE_0	5
+
 /* Note header in a PT_NOTE section */
 typedef struct elf32_note {
   Elf32_Word	n_namesz;	/* Name size */
-- 
2.1.4

Re: [RFC PATCH 2/2] ELF: Add ELF program property parsing support

[Yu-cheng Yu]

On Tue, 2019-08-20 at 10:57 +0100, Dave Martin wrote:
> ELF program properties will needed for detecting whether to enable
> optional architecture or ABI features for a new ELF process.
> 
> For now, there are no generic properties that we care about, so do
> nothing unless CONFIG_ARCH_USE_GNU_PROPERTY=y.
> 
> Otherwise, the presence of properties using the PT_PROGRAM_PROPERTY
> phdrs entry (if any), and notify each property to the arch code.
> 
> For now, the added code is not used.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> ---
>  fs/binfmt_elf.c          | 109
> +++++++++++++++++++++++++++++++++++++++++++++++
>  fs/compat_binfmt_elf.c   |   4 ++
>  include/linux/elf.h      |  21 +++++++++
>  include/uapi/linux/elf.h |   4 ++
>  4 files changed, 138 insertions(+)
> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index d4e11b2..52f4b96 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -39,12 +39,18 @@
>  #include <linux/sched/coredump.h>
>  #include <linux/sched/task_stack.h>
>  #include <linux/sched/cputime.h>
> +#include <linux/sizes.h>
> +#include <linux/types.h>
>  #include <linux/cred.h>
>  #include <linux/dax.h>
>  #include <linux/uaccess.h>
>  #include <asm/param.h>
>  #include <asm/page.h>
>  
> +#ifndef ELF_COMPAT
> +#define ELF_COMPAT 0
> +#endif
> +
>  #ifndef user_long_t
>  #define user_long_t long
>  #endif
> @@ -690,6 +696,93 @@ static unsigned long randomize_stack_top(unsigned long
> stack_top)
>  #endif
>  }
>  
> +static int parse_elf_property(const void **prop, size_t *notesz,
> +			      struct arch_elf_state *arch)
> +{
> +	const struct gnu_property *pr = *prop;
> +	size_t sz = *notesz;
> +	int ret;
> +	size_t step;
> +
> +	BUG_ON(sz < sizeof(*pr));
> +
> +	if (sizeof(*pr) > sz)
> +		return -EIO;
> +
> +	if (pr->pr_datasz > sz - sizeof(*pr))
> +		return -EIO;
> +
> +	step = round_up(sizeof(*pr) + pr->pr_datasz, elf_gnu_property_align);
> +	if (step > sz)
> +		return -EIO;
> +
> +	ret = arch_parse_elf_property(pr->pr_type, *prop + sizeof(*pr),
> +				      pr->pr_datasz, ELF_COMPAT, arch);
> +	if (ret)
> +		return ret;
> +
> +	*prop += step;
> +	*notesz -= step;
> +	return 0;
> +}
> +
> +#define NOTE_DATA_SZ SZ_1K
> +#define GNU_PROPERTY_TYPE_0_NAME "GNU"
> +#define NOTE_NAME_SZ (sizeof(GNU_PROPERTY_TYPE_0_NAME))
> +
> +static int parse_elf_properties(struct file *f, const struct elf_phdr *phdr,
> +				struct arch_elf_state *arch)
> +{
> +	ssize_t n;
> +	loff_t pos = phdr->p_offset;
> +	union {
> +		struct elf_note nhdr;
> +		char data[NOTE_DATA_SZ];
> +	} note;
> +	size_t off, notesz;
> +	const void *prop;
> +	int ret;
> +
> +	if (!IS_ENABLED(ARCH_USE_GNU_PROPERTY))
> +		return 0;
> +
> +	BUG_ON(phdr->p_type != PT_GNU_PROPERTY);
> +
> +	/* If the properties are crazy large, that's too bad (for now): */
> +	if (phdr->p_filesz > sizeof(note))
> +		return -ENOEXEC;
> +	n = kernel_read(f, &note, phdr->p_filesz, &pos);
> +
> +	BUILD_BUG_ON(sizeof(note) < sizeof(note.nhdr) + NOTE_NAME_SZ);
> +	if (n < 0 || n < sizeof(note.nhdr) + NOTE_NAME_SZ)
> +		return -EIO;
> +
> +	if (note.nhdr.n_type != NT_GNU_PROPERTY_TYPE_0 ||
> +	    note.nhdr.n_namesz != NOTE_NAME_SZ ||
> +	    strncmp(note.data + sizeof(note.nhdr),
> +		    GNU_PROPERTY_TYPE_0_NAME, n - sizeof(note.nhdr)))
> +		return -EIO;
> +
> +	off = round_up(sizeof(note.nhdr) + NOTE_NAME_SZ,
> +		       elf_gnu_property_align);
> +	if (off > n)
> +		return -EIO;
> +
> +	prop = (const struct gnu_property *)(note.data + off);
> +	notesz = n - off;
> +	if (note.nhdr.n_descsz > notesz)
> +		return -EIO;
> +
> +	while (notesz) {
> +		BUG_ON(((char *)prop - note.data) % elf_gnu_property_align);
> +		ret = parse_elf_property(&prop, &notesz, arch);

Properties need to be in ascending order.  Can we keep track of it from here.
Also, can we replace BUG_ON with returning an error.

Thanks,
Yu-cheng

Re: [RFC PATCH 2/2] ELF: Add ELF program property parsing support

[Dave Martin]

On Tue, Aug 20, 2019 at 10:40:54PM +0100, Yu-cheng Yu wrote:
> On Tue, 2019-08-20 at 10:57 +0100, Dave Martin wrote:
> > ELF program properties will needed for detecting whether to enable
> > optional architecture or ABI features for a new ELF process.
> > 
> > For now, there are no generic properties that we care about, so do
> > nothing unless CONFIG_ARCH_USE_GNU_PROPERTY=y.
> > 
> > Otherwise, the presence of properties using the PT_PROGRAM_PROPERTY
> > phdrs entry (if any), and notify each property to the arch code.
> > 
> > For now, the added code is not used.
> > 
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > ---
> >  fs/binfmt_elf.c          | 109
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  fs/compat_binfmt_elf.c   |   4 ++
> >  include/linux/elf.h      |  21 +++++++++
> >  include/uapi/linux/elf.h |   4 ++
> >  4 files changed, 138 insertions(+)
> > 
> > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> > index d4e11b2..52f4b96 100644
> > --- a/fs/binfmt_elf.c
> > +++ b/fs/binfmt_elf.c
> > @@ -39,12 +39,18 @@
> >  #include <linux/sched/coredump.h>
> >  #include <linux/sched/task_stack.h>
> >  #include <linux/sched/cputime.h>
> > +#include <linux/sizes.h>
> > +#include <linux/types.h>
> >  #include <linux/cred.h>
> >  #include <linux/dax.h>
> >  #include <linux/uaccess.h>
> >  #include <asm/param.h>
> >  #include <asm/page.h>
> >  
> > +#ifndef ELF_COMPAT
> > +#define ELF_COMPAT 0
> > +#endif
> > +
> >  #ifndef user_long_t
> >  #define user_long_t long
> >  #endif
> > @@ -690,6 +696,93 @@ static unsigned long randomize_stack_top(unsigned long
> > stack_top)
> >  #endif
> >  }
> >  
> > +static int parse_elf_property(const void **prop, size_t *notesz,
> > +			      struct arch_elf_state *arch)
> > +{
> > +	const struct gnu_property *pr = *prop;
> > +	size_t sz = *notesz;
> > +	int ret;
> > +	size_t step;
> > +
> > +	BUG_ON(sz < sizeof(*pr));
> > +
> > +	if (sizeof(*pr) > sz)
> > +		return -EIO;
> > +
> > +	if (pr->pr_datasz > sz - sizeof(*pr))
> > +		return -EIO;
> > +
> > +	step = round_up(sizeof(*pr) + pr->pr_datasz, elf_gnu_property_align);
> > +	if (step > sz)
> > +		return -EIO;
> > +
> > +	ret = arch_parse_elf_property(pr->pr_type, *prop + sizeof(*pr),
> > +				      pr->pr_datasz, ELF_COMPAT, arch);
> > +	if (ret)
> > +		return ret;
> > +
> > +	*prop += step;
> > +	*notesz -= step;
> > +	return 0;
> > +}
> > +
> > +#define NOTE_DATA_SZ SZ_1K
> > +#define GNU_PROPERTY_TYPE_0_NAME "GNU"
> > +#define NOTE_NAME_SZ (sizeof(GNU_PROPERTY_TYPE_0_NAME))
> > +
> > +static int parse_elf_properties(struct file *f, const struct elf_phdr *phdr,
> > +				struct arch_elf_state *arch)
> > +{
> > +	ssize_t n;
> > +	loff_t pos = phdr->p_offset;
> > +	union {
> > +		struct elf_note nhdr;
> > +		char data[NOTE_DATA_SZ];
> > +	} note;
> > +	size_t off, notesz;
> > +	const void *prop;
> > +	int ret;
> > +
> > +	if (!IS_ENABLED(ARCH_USE_GNU_PROPERTY))
> > +		return 0;
> > +
> > +	BUG_ON(phdr->p_type != PT_GNU_PROPERTY);
> > +
> > +	/* If the properties are crazy large, that's too bad (for now): */
> > +	if (phdr->p_filesz > sizeof(note))
> > +		return -ENOEXEC;
> > +	n = kernel_read(f, &note, phdr->p_filesz, &pos);
> > +
> > +	BUILD_BUG_ON(sizeof(note) < sizeof(note.nhdr) + NOTE_NAME_SZ);
> > +	if (n < 0 || n < sizeof(note.nhdr) + NOTE_NAME_SZ)
> > +		return -EIO;
> > +
> > +	if (note.nhdr.n_type != NT_GNU_PROPERTY_TYPE_0 ||
> > +	    note.nhdr.n_namesz != NOTE_NAME_SZ ||
> > +	    strncmp(note.data + sizeof(note.nhdr),
> > +		    GNU_PROPERTY_TYPE_0_NAME, n - sizeof(note.nhdr)))
> > +		return -EIO;
> > +
> > +	off = round_up(sizeof(note.nhdr) + NOTE_NAME_SZ,
> > +		       elf_gnu_property_align);
> > +	if (off > n)
> > +		return -EIO;
> > +
> > +	prop = (const struct gnu_property *)(note.data + off);
> > +	notesz = n - off;
> > +	if (note.nhdr.n_descsz > notesz)
> > +		return -EIO;
> > +
> > +	while (notesz) {
> > +		BUG_ON(((char *)prop - note.data) % elf_gnu_property_align);
> > +		ret = parse_elf_property(&prop, &notesz, arch);
> 
> Properties need to be in ascending order.  Can we keep track of it from here.

We could, but do we need to?  If this order is violated, the ELF file is
invalid and it doesn't matter what we do, providing that the kernel
doesn't go wrong.

In general, the ELF loader already doesn't try to detect invalid ELF
files: for example EM_386 with ELFCLASS64 would just be executed as a
32-bit binary.  Of course, if the file is really structured as a 64-bit
ELF we'll probably fail to parse the file before we get as far as
executing it.

Here, we just care that a particular property is there.  If the
properties are shuffled, we will find the same set of properties
regardless.

The kernel isn't really responsible for debugging broken linkers...

OTOH the check would be trivial and I don't have a strong objection to
adding it.

> Also, can we replace BUG_ON with returning an error.

Sure, those BUG_ON() are for development purposes only.  I'd intended to
remove them, but I forgot to comment on it.

This BUG_ON() should be ensured by the round_up() logic in
parse_elf_property().

Thanks for taking a look!

Cheers
---Dave