On Wed, May 30, 2018 at 11:48:59AM +0100, Dave Martin wrote:

On Tue, May 29, 2018 at 09:42:31PM +0100, Will Deacon wrote:

quoted

On Fri, May 25, 2018 at 04:17:08PM +0100, Dave Martin wrote:

quoted

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index fac1c4d..8cf112b 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h

@@ -121,6 +121,9 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/bug.h>
+#include <asm/processor.h> /* for signal_minsigstksz, used by ARCH_DLINFO */
+
 typedef unsigned long elf_greg_t;
 
 #define ELF_NGREG (sizeof(struct user_pt_regs) / sizeof(elf_greg_t))

@@ -148,6 +151,14 @@ typedef struct user_fpsimd_state elf_fpregset_t;
 do {									\
 	NEW_AUX_ENT(AT_SYSINFO_EHDR,					\
 		    (elf_addr_t)current->mm->context.vdso);		\
+									\
+	/*								\
+	 * Should always be nonzero unless there's a kernel bug.	\
+	 * If we haven't determined a sensible value to give to		\
+	 * userspace, omit the entry:					\
+	 */								\
+	if (likely(signal_minsigstksz))					\
+		NEW_AUX_ENT(AT_MINSIGSTKSZ, signal_minsigstksz);	\
 } while (0)

I think this is the desired behaviour, but now I'm worried that we're forced
to have AT_VECTOR_SIZE_ARCH defined as 2 and, whilst you're correct that the
ELF loader deals with this gracefuly, the FDPIC loader looks a lot less
robust (in particular, my reading is that it decrements the stack pointer
and then pushes these entries in reverse order by overloading NEW_AUX_ENT).

config BINFMT_ELF_FDPIC
	/* ... */
	depends on (ARM || (SUPERH32 & !MMU) || C6X)

Ok, that's a relief. The checkpoint stuff is still a bit worrying though
(prctl_set_mm_map).

The FDPIC loader seems to assume it's 32-bit only and also looks broken
with regard to auxv:

	/* force 16 byte _final_ alignment here for generality */
#define DLINFO_ITEMS 15

/* ... */

		nr = 0;
		csp -= 2 * sizeof(unsigned long);
		NEW_AUX_ENT(AT_EXECFD, ...);
	}

/* ... */

	csp -= DLINFO_ITEMS * 2 * sizeof(unsigned long);
	NEW_AUX_ENT(AT_HWCAP,   ELF_HWCAP);
#ifdef ELF_HWCAP2
	NEW_AUX_ENT(AT_HWCAP2,  ELF_HWCAP2);
#endif
	/* 14 more NEW_AUX_ENT() */


Looks like commit 2171364d1a92 ("powerpc: Add HWCAP2 aux entry") added
HWCAP2 without ensuring that space is reserved.

I can try to draft a patch to handle the auxv in a more sane way for
FDPIC, but either way I don't see that it should be relevant to arm64.


AT_IGNORE feels like a bit of a fig leaf, but it's harmless enough.  I'm
happy to add it if you prefer.

The only userspace code I could find that uses it is something that prints
out auxv, but I'd still better spitting it out so we don't have to worry
about being smaller than AT_VECTOR_SIZE_ARCH.

Thanks,

Will