saner elf compat

This series deals with the warts in ELF compat on triarch
architectures (x86_64 and mips64, that is).

x86_64 at least does use compat_binfmt_elf.c for both
32bit ABIs; the way it is done is ugly as hell, though, and more than slightly brittle (see asm/compat.h for PRSTATUS_SIZE and SET_PR_FPVALID definitions - IMO that kind of magic is too ugly to live).

mips64, OTOH, does not use compat_binfmt_elf.c for either of its
32bit ABIs. It has a couple of analogues (each with include of ../../../fs/binfmt_elf.c, BTW), with quite a bit of ancient cruft accumulated in those.

Fortunately, cleanup of i386/x32 mess (first 3 commits in
the series) provides a fairly straightforward way for mips64 to use fs/compat_binfmt_elf.c for both n32 and o32.

That stuff had been sitting around since June; lately rdd has
spotted Kconfig problems around COMPAT_BINFMT_ELF selects. All of them had been on configs that had COMPAT_BINFMT_ELF != COMPAT && BINFMT_ELF. For most of the architectures that's impossible to achieve, but some (sparc, e.g.) can end up with that. Randy posted a patch adding if BINFMT_ELF to selects that lacked it, but that looked wrong to me - why not centralize that logics into fs/Kconfig.binfmt? IOW, what's the point of having any such selects in arch/*/Kconfig?

The answer (for mainline) is that mips compat does *NOT* want
COMPAT_BINFMT_ELF. Not a problem with that series, though, so I'd retested it (seems to work, both for x86_64 and mips64, execs and coredumps for all ABIs alike), with centralization of Kconfig logics thrown in.

It's based at 5.10-rc1 and lives in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git#work.elf-compat
I'll post the individual patches in followups.

Shortlog:
binfmt_elf: partially sanitize PRSTATUS_SIZE and SET_PR_FPVALID
elf_prstatus: collect the common part (everything before pr_reg) into a struct [elfcore-compat][amd64] clean PRSTATUS_SIZE/SET_PR_FPVALID up properly mips binfmt_elf*32.c: use elfcore-compat.h
mips: kill unused definitions in binfmt_elf[on]32.c
mips: KVM_GUEST makes no sense for 64bit builds...
mips compat: don't bother with ELF_ET_DYN_BASE
mips: don't bother with ELF_CORE_EFLAGS
mips compat: switch to compat_binfmt_elf.c
Kconfig: regularize selection of CONFIG_BINFMT_ELF

Diffstat:

 arch/Kconfig                               |   3 +
 arch/arm64/Kconfig                         |   1 -
 arch/ia64/kernel/crash.c                   |   2 +-
 arch/mips/Kconfig                          |   8 +--
 arch/mips/include/asm/elf.h                |  56 +++++----------
 arch/mips/include/asm/elfcore-compat.h     |  29 ++++++++
 arch/mips/kernel/Makefile                  |   4 +-
 arch/mips/kernel/binfmt_elfn32.c           | 106 ----------------------------
 arch/mips/kernel/binfmt_elfo32.c           | 109 -----------------------------
 arch/mips/kernel/scall64-n64.S             |   2 +-
 arch/parisc/Kconfig                        |   1 -
 arch/powerpc/Kconfig                       |   1 -
 arch/powerpc/platforms/powernv/opal-core.c |   6 +-
 arch/s390/Kconfig                          |   1 -
 arch/s390/kernel/crash_dump.c              |   2 +-
 arch/sparc/Kconfig                         |   1 -
 arch/x86/Kconfig                           |   2 +-
 arch/x86/include/asm/compat.h              |  11 ---
 arch/x86/include/asm/elfcore-compat.h      |  31 ++++++++
 fs/Kconfig.binfmt                          |   2 +-
 fs/binfmt_elf.c                            |  19 +++--
 fs/binfmt_elf_fdpic.c                      |  22 ++----
 fs/compat_binfmt_elf.c                     |   1 +
 include/linux/elfcore-compat.h             |  15 +++-
 include/linux/elfcore.h                    |   7 +-
 kernel/kexec_core.c                        |   2 +-
 26 files changed, 127 insertions(+), 317 deletions(-)
 create mode 100644 arch/mips/include/asm/elfcore-compat.h
 delete mode 100644 arch/mips/kernel/binfmt_elfn32.c
 delete mode 100644 arch/mips/kernel/binfmt_elfo32.c
 create mode 100644 arch/x86/include/asm/elfcore-compat.h

From: Al Viro <[email protected]>

On 64bit architectures that support 32bit processes there are two possible layouts for NT_PRSTATUS note in ELF coredumps. For one thing, several fields are 64bit for native processes and 32bit for compat ones (pr_sigpend, etc.). For another, the register dump is obviously different - the size and number of registers are not going to be the same for 32bit and 64bit variants of processor.

Usually that's handled by having two structures - elf_prstatus for native layout and compat_elf_prstatus for 32bit one.
32bit processes are handled by fs/compat_binfmt_elf.c, which defines a macro called 'elf_prstatus' that expands to compat_elf_prstatus. Then it includes fs/binfmt_elf.c, which makes all references to struct elf_prstatus to be textually replaced with struct compat_elf_prstatus. Ugly and somewhat brittle, but it works.

However, amd64 is worse - there are _three_ possible layouts. One for native 64bit processes, another for i386 (32bit) processes and yet another for x32 (32bit address space with full 64bit registers).

Both i386 and x32 processes are handled by fs/compat_binfmt_elf.c, with usual compat_binfmt_elf.c trickery. However, the layouts for i386 and x32 are not identical - they have the common beginning, but the register dump part (pr_reg) is bigger on x32. Worse, pr_reg is not the last field - it's followed by int pr_fpvalid, so that field ends up at different offsets for i386 and x32 layouts.

Fortunately, there's not much code that cares about any of that - it's all encapsulated in fill_thread_core_info(). Since x32 variant is bigger, we define compat_elf_prstatus to match that layout. That way i386 processes have enough space to fit their layout into.

Moreover, since these layouts are identical prior to pr_reg, we don't need to distinguish x32 and i386 cases when we are setting the fields prior to pr_reg.

Filling pr_reg itself is done by calling ->get() method of appropriate regset, and that method knows what layout (and size) to use.

We do need to distinguish x32 and i386 cases only for two
things: setting ->pr_fpvalid (offset differs for x32 and i386) and choosing the right size for our note.

The way it's done is Not Nice, for the lack of more accurate printable description. There are two macros (PRSTATUS_SIZE and SET_PR_FPVALID), that default essentially to sizeof(struct elf_prstatus) and (S)->pr_fpvalid = 1. On x86 asm/compat.h provides its own variants.

Unfortunately, quite a few things go wrong there:
* PRSTATUS_SIZE doesn't use the normal test for process
being an x32 one (TIF_X32); it compares the size reported by regset with the size of pr_reg.
* it hardcodes the sizes of x32 and i386 variants (296 and 144
resp.), so if some change in includes leads to asm/compat.h pulled in by fs/binfmt_elf.c we are in trouble - it will end up using the size of x32 variant for 64bit processes.
* it's in the wrong place; asm/compat.h couldn't define
the structure for i386 layout, since it lacks quite a few types needed for it. Hardcoded sizes are largely due to that.

The proper fix would be to have an explicitly defined i386 variant of structure and have PRSTATUS_SIZE/SET_PR_FPVALID check for TIF_X32 to choose the variant that should be used. Unfortunately, that requires some manipulations of headers; we'll do that later in the series, but for now let's go with the minimal variant - rename PRSTATUS_SIZE in asm/compat.h to COMPAT_PRSTATUS_SIZE, have fs/compat_binfmt_elf.c define PRSTATUS_SIZE to COMPAT_PRSTATUS_SIZE and use the normal TIF_X32 check in that macro. The size of i386 variant is kept hardcoded for now. Similar story for SET_PR_FPVALID.

---
 arch/x86/include/asm/compat.h | 11 +++++++----
 fs/binfmt_elf.c               | 13 +++++--------
 fs/compat_binfmt_elf.c        |  8 ++++++++
 3 files changed, 20 insertions(+), 12 deletions(-)

index 0e327a01f50f..1897e1dcbdd2 100644
@@ -165,10 +165,13 @@ struct compat_shmid64_ds {
 typedef struct user_regs_struct compat_elf_gregset_t;
 
 /* Full regset -- prstatus on x32, otherwise on ia32 */
-#define PRSTATUS_SIZE(S, R) (R != sizeof(S.pr_reg) ? 144 : 296)
-#define SET_PR_FPVALID(S, V, R) \
-  do { *(int *) (((void *) &((S)->pr_reg)) + R) = (V); } \
-  while (0)
+#define COMPAT_PRSTATUS_SIZE (test_thread_flag(TIF_X32) \
+	? sizeof(struct compat_elf_prstatus) \
+	: 144)
+#define COMPAT_SET_PR_FPVALID(S) \
+	(*(test_thread_flag(TIF_X32)	\
+	       ? &(S)->pr_fpvalid	\
+               : (int *)((void *)(S) + 140)) = 1)
 
 #ifdef CONFIG_X86_X32_ABI
 #define COMPAT_USE_64BIT_TIME \

index b6b3d052ca86..f066882bd270 100644
@@ -1717,11 +1717,11 @@ static void do_thread_regset_writeback(struct task_struct *task,
 }
 
 #ifndef PRSTATUS_SIZE
-#define PRSTATUS_SIZE(S, R) sizeof(S)
+#define PRSTATUS_SIZE sizeof(struct elf_prstatus)
 #endif
 
 #ifndef SET_PR_FPVALID
-#define SET_PR_FPVALID(S, V, R) ((S)->pr_fpvalid = (V))
+#define SET_PR_FPVALID(S) ((S)->pr_fpvalid = 1)
 #endif
 
 static int fill_thread_core_info(struct elf_thread_core_info *t,

@@ -1729,7 +1729,6 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
 				 long signr, size_t *total)
 {
 	unsigned int i;
-	int regset0_size;
 
 	/*
 	 * NT_PRSTATUS is the one special case, because the regset data

@@ -1738,13 +1737,11 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
 	 * We assume that regset 0 is NT_PRSTATUS.
 	 */
 	fill_prstatus(&t->prstatus, t->task, signr);
-	regset0_size = regset_get(t->task, &view->regsets[0],
+	regset_get(t->task, &view->regsets[0],
 		   sizeof(t->prstatus.pr_reg), &t->prstatus.pr_reg);
-	if (regset0_size < 0)
-		return 0;
 
 	fill_note(&t->notes[0], "CORE", NT_PRSTATUS,
-		  PRSTATUS_SIZE(t->prstatus, regset0_size), &t->prstatus);
+		  PRSTATUS_SIZE, &t->prstatus);
 	*total += notesize(&t->notes[0]);
 
 	do_thread_regset_writeback(t->task, &view->regsets[0]);

@@ -1772,7 +1769,7 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
 			continue;
 
 		if (is_fpreg)
-			SET_PR_FPVALID(&t->prstatus, 1, regset0_size);
+			SET_PR_FPVALID(&t->prstatus);
 
 		fill_note(&t->notes[i], is_fpreg ? "CORE" : "LINUX",
 			  note_type, ret, data);

index 2d24c765cbd7..13cd78773700 100644
@@ -95,6 +95,14 @@
 #define	ELF_EXEC_PAGESIZE	COMPAT_ELF_EXEC_PAGESIZE
 #endif
 
+#ifdef	COMPAT_PRSTATUS_SIZE
+#define	PRSTATUS_SIZE COMPAT_PRSTATUS_SIZE
+#endif
+
+#ifdef	COMPAT_SET_PR_FPVALID
+#define	SET_PR_FPVALID(S) COMPAT_SET_PR_FPVALID(S)
+#endif
+
 #ifdef	COMPAT_ELF_PLAT_INIT
 #undef	ELF_PLAT_INIT
 #define	ELF_PLAT_INIT		COMPAT_ELF_PLAT_INIT

2.11.0

From: Al Viro <[email protected]>

Preparations to doing i386 compat elf_prstatus sanely - rather than duplicating the beginning of compat_elf_prstatus, take these fields into a separate structure (compat_elf_prstatus_common), so that it could be reused. Due to the incestous relationship between binfmt_elf.c and compat_binfmt_elf.c we need the same shape change done to native struct elf_prstatus, gathering the fields prior to pr_reg into a new structure (struct elf_prstatus_common).

Fortunately, offset of pr_reg is always a multiple of 16 with no padding right before it, so it's possible to turn all the stuff prior to it into a single member without disturbing the layout.

---
 arch/ia64/kernel/crash.c                   |  2 +-
 arch/mips/kernel/binfmt_elfn32.c           |  7 ++++++-
 arch/mips/kernel/binfmt_elfo32.c           |  6 +++++-
 arch/powerpc/platforms/powernv/opal-core.c |  6 +++---
 arch/s390/kernel/crash_dump.c              |  2 +-
 fs/binfmt_elf.c                            |  6 +++---
 fs/binfmt_elf_fdpic.c                      | 22 +++++-----------------
 fs/compat_binfmt_elf.c                     |  1 +
 include/linux/elfcore-compat.h             |  7 ++++++-
 include/linux/elfcore.h                    |  7 ++++++-
 kernel/kexec_core.c                        |  2 +-
 11 files changed, 38 insertions(+), 30 deletions(-)

index fec70d662d0c..4f47741005d2 100644
@@ -43,7 +43,7 @@ crash_save_this_cpu(void)
 
 	elf_greg_t *dst = (elf_greg_t *)&(prstatus->pr_reg);
 	memset(prstatus, 0, sizeof(*prstatus));
-	prstatus->pr_pid = current->pid;
+	prstatus->common.pr_pid = current->pid;
 
 	ia64_dump_cpu_regs(dst);
 	cfm = dst[43];

index 6ee3f7218c67..136dc0c9300d 100644
@@ -44,7 +44,8 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
 #include <linux/math64.h>
 
 #define elf_prstatus elf_prstatus32
-struct elf_prstatus32
+#define elf_prstatus_common elf_prstatus32_common
+struct elf_prstatus32_common
 {
 	struct elf_siginfo pr_info;	/* Info associated with signal */
 	short	pr_cursig;		/* Current signal */

@@ -58,6 +59,10 @@ struct elf_prstatus32
 	struct old_timeval32 pr_stime; /* System time */
 	struct old_timeval32 pr_cutime;/* Cumulative user time */
 	struct old_timeval32 pr_cstime;/* Cumulative system time */
+};
+struct elf_prstatus32
+{
+	struct elf_prstatus32_common common:
 	elf_gregset_t pr_reg;	/* GP registers */
 	int pr_fpvalid;		/* True if math co-processor being used.  */
 };

index 6dd103d3cebb..b1f4b8f1dee7 100644
@@ -49,7 +49,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
 #include <linux/math64.h>
 
 #define elf_prstatus elf_prstatus32
-struct elf_prstatus32
+struct elf_prstatus32_common
 {
 	struct elf_siginfo pr_info;	/* Info associated with signal */
 	short	pr_cursig;		/* Current signal */

@@ -63,6 +63,10 @@ struct elf_prstatus32
 	struct old_timeval32 pr_stime; /* System time */
 	struct old_timeval32 pr_cutime;/* Cumulative user time */
 	struct old_timeval32 pr_cstime;/* Cumulative system time */
+};
+struct elf_prstatus32
+{
+	struct elf_prstatus32_common common:
 	elf_gregset_t pr_reg;	/* GP registers */
 	int pr_fpvalid;		/* True if math co-processor being used.  */
 };

index 23571f0b555a..0d9ba70f7251 100644
@@ -119,8 +119,8 @@ static void fill_prstatus(struct elf_prstatus *prstatus, int pir,
 	 * As a PIR value could also be '0', add an offset of '100'
 	 * to every PIR to avoid misinterpretations in GDB.
 	 */
-	prstatus->pr_pid  = cpu_to_be32(100 + pir);
-	prstatus->pr_ppid = cpu_to_be32(1);
+	prstatus->common.pr_pid  = cpu_to_be32(100 + pir);
+	prstatus->common.pr_ppid = cpu_to_be32(1);
 
 	/*
 	 * Indicate SIGUSR1 for crash initiated from kernel.

@@ -130,7 +130,7 @@ static void fill_prstatus(struct elf_prstatus *prstatus, int pir,
 		short sig;
 
 		sig = kernel_initiated ? SIGUSR1 : SIGTERM;
-		prstatus->pr_cursig = cpu_to_be16(sig);
+		prstatus->common.pr_cursig = cpu_to_be16(sig);
 	}
 }
 

index 205b2e2648aa..0e36dfc9ccd6 100644
@@ -365,7 +365,7 @@ static void *fill_cpu_elf_notes(void *ptr, int cpu, struct save_area *sa)
 	memcpy(&nt_prstatus.pr_reg.gprs, sa->gprs, sizeof(sa->gprs));
 	memcpy(&nt_prstatus.pr_reg.psw, sa->psw, sizeof(sa->psw));
 	memcpy(&nt_prstatus.pr_reg.acrs, sa->acrs, sizeof(sa->acrs));
-	nt_prstatus.pr_pid = cpu;
+	nt_prstatus.common.pr_pid = cpu;
 	/* Prepare fpregset (floating point) note */
 	memset(&nt_fpregset, 0, sizeof(nt_fpregset));
 	memcpy(&nt_fpregset.fpc, &sa->fpc, sizeof(sa->fpc));

index f066882bd270..c76324f647ae 100644
@@ -1495,7 +1495,7 @@ static void fill_note(struct memelfnote *note, const char *name, int type,
  * fill up all the fields in prstatus from the given task struct, except
  * registers which need to be filled up separately.
  */
-static void fill_prstatus(struct elf_prstatus *prstatus,
+static void fill_prstatus(struct elf_prstatus_common *prstatus,
 		struct task_struct *p, long signr)
 {
 	prstatus->pr_info.si_signo = prstatus->pr_cursig = signr;

@@ -1736,7 +1736,7 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
 	 * than being the whole note contents.  We fill the reset in here.
 	 * We assume that regset 0 is NT_PRSTATUS.
 	 */
-	fill_prstatus(&t->prstatus, t->task, signr);
+	fill_prstatus(&t->prstatus.common, t->task, signr);
 	regset_get(t->task, &view->regsets[0],
 		   sizeof(t->prstatus.pr_reg), &t->prstatus.pr_reg);
 

@@ -1958,7 +1958,7 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
 	struct task_struct *p = t->thread;
 	t->num_notes = 0;
 
-	fill_prstatus(&t->prstatus, p, signr);
+	fill_prstatus(&t->prstatus.common, p, signr);
 	elf_core_copy_task_regs(p, &t->prstatus.pr_reg);	
 	
 	fill_note(&t->notes[0], "CORE", NT_PRSTATUS, sizeof(t->prstatus),

index be4062b8ba75..03d81a14bcbf 100644
@@ -1191,18 +1191,7 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *params,
 
 struct elf_prstatus_fdpic
 {
-	struct elf_siginfo pr_info;	/* Info associated with signal */
-	short	pr_cursig;		/* Current signal */
-	unsigned long pr_sigpend;	/* Set of pending signals */
-	unsigned long pr_sighold;	/* Set of held signals */
-	pid_t	pr_pid;
-	pid_t	pr_ppid;
-	pid_t	pr_pgrp;
-	pid_t	pr_sid;
-	struct __kernel_old_timeval pr_utime;	/* User time */
-	struct __kernel_old_timeval pr_stime;	/* System time */
-	struct __kernel_old_timeval pr_cutime;	/* Cumulative user time */
-	struct __kernel_old_timeval pr_cstime;	/* Cumulative system time */
+	struct elf_prstatus_common	common;
 	elf_gregset_t pr_reg;	/* GP registers */
 	/* When using FDPIC, the loadmap addresses need to be communicated
 	 * to GDB in order for GDB to do the necessary relocations.  The

@@ -1301,7 +1290,7 @@ static inline void fill_note(struct memelfnote *note, const char *name, int type
  * fill up all the fields in prstatus from the given task struct, except
  * registers which need to be filled up separately.
  */
-static void fill_prstatus(struct elf_prstatus_fdpic *prstatus,
+static void fill_prstatus(struct elf_prstatus_common *prstatus,
 			  struct task_struct *p, long signr)
 {
 	prstatus->pr_info.si_signo = prstatus->pr_cursig = signr;

@@ -1332,9 +1321,6 @@ static void fill_prstatus(struct elf_prstatus_fdpic *prstatus,
 	}
 	prstatus->pr_cutime = ns_to_kernel_old_timeval(p->signal->cutime);
 	prstatus->pr_cstime = ns_to_kernel_old_timeval(p->signal->cstime);
-
-	prstatus->pr_exec_fdpic_loadmap = p->mm->context.exec_fdpic_loadmap;
-	prstatus->pr_interp_fdpic_loadmap = p->mm->context.interp_fdpic_loadmap;
 }
 
 static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,

@@ -1405,7 +1391,9 @@ static struct elf_thread_status *elf_dump_thread_status(long signr, struct task_
 	if (!t)
 		return t;
 
-	fill_prstatus(&t->prstatus, p, signr);
+	fill_prstatus(&t->prstatus.common, p, signr);
+	t->prstatus.pr_exec_fdpic_loadmap = p->mm->context.exec_fdpic_loadmap;
+	t->prstatus.pr_interp_fdpic_loadmap = p->mm->context.interp_fdpic_loadmap;
 	regset_get(p, &view->regsets[0],
 		   sizeof(t->prstatus.pr_reg), &t->prstatus.pr_reg);
 

index 13cd78773700..a40df32acff5 100644
@@ -50,6 +50,7 @@
  * which requires asm/elf.h to define compat_elf_gregset_t et al.
  */
 #define elf_prstatus	compat_elf_prstatus
+#define elf_prstatus_common	compat_elf_prstatus_common
 #define elf_prpsinfo	compat_elf_prpsinfo
 
 #undef ns_to_kernel_old_timeval

index 10485f0c9740..4aeda5f1f038 100644
@@ -17,7 +17,7 @@ struct compat_elf_siginfo
 	compat_int_t			si_errno;
 };
 
-struct compat_elf_prstatus
+struct compat_elf_prstatus_common
 {
 	struct compat_elf_siginfo	pr_info;
 	short				pr_cursig;

@@ -31,6 +31,11 @@ struct compat_elf_prstatus
 	struct old_timeval32		pr_stime;
 	struct old_timeval32		pr_cutime;
 	struct old_timeval32		pr_cstime;
+};
+
+struct compat_elf_prstatus
+{
+	struct compat_elf_prstatus_common	common;
 	compat_elf_gregset_t		pr_reg;
 	compat_int_t			pr_fpvalid;
 };

index 46c3d691f677..641a9041182d 100644
@@ -29,7 +29,7 @@ struct elf_siginfo
  * the SVR4 structure, but more Linuxy, with things that Linux does
  * not support and which gdb doesn't really use excluded.
  */
-struct elf_prstatus
+struct elf_prstatus_common
 {
 	struct elf_siginfo pr_info;	/* Info associated with signal */
 	short	pr_cursig;		/* Current signal */

@@ -43,6 +43,11 @@ struct elf_prstatus
 	struct __kernel_old_timeval pr_stime;	/* System time */
 	struct __kernel_old_timeval pr_cutime;	/* Cumulative user time */
 	struct __kernel_old_timeval pr_cstime;	/* Cumulative system time */
+};
+
+struct elf_prstatus
+{
+	struct elf_prstatus_common common;
 	elf_gregset_t pr_reg;	/* GP registers */
 	int pr_fpvalid;		/* True if math co-processor being used.  */
 };

index 8798a8183974..e61ab826ab18 100644
@@ -1077,7 +1077,7 @@ void crash_save_cpu(struct pt_regs *regs, int cpu)
 	if (!buf)
 		return;
 	memset(&prstatus, 0, sizeof(prstatus));
-	prstatus.pr_pid = current->pid;
+	prstatus.common.pr_pid = current->pid;
 	elf_core_copy_kernel_regs(&prstatus.pr_reg, regs);
 	buf = append_elf_note(buf, KEXEC_CORE_NOTE_NAME, NT_PRSTATUS,
 			      &prstatus, sizeof(prstatus));

2.11.0

From: Al Viro <[email protected]>

To get rid of hardcoded size/offset in those macros we need to have a definition of i386 variant of struct elf_prstatus. However, we can't do that in asm/compat.h - the types needed for that are not there and adding an include of asm/user32.h into asm/compat.h would cause a lot of mess.

That could be conveniently done in elfcore-compat.h, but currently there is nowhere to put arch-dependent parts of it - no asm/elfcore-compat.h. So we introduce a new file (asm/elfcore-compat.h, present on architectures that have CONFIG_ARCH_HAS_ELFCORE_COMPAT set, currently only on x86), have it pulled by linux/elfcore-compat.h and move the definitions there.

As a side benefit, we don't need to worry about accidental inclusion of that file into binfmt_elf.c itself, so we don't need the dance with COMPAT_PRSTATUS_SIZE, etc. - only fs/compat_binfmt_elf.c will see that header.

---
 arch/Kconfig                          |  3 +++
 arch/x86/Kconfig                      |  1 +
 arch/x86/include/asm/compat.h         | 14 --------------
 arch/x86/include/asm/elfcore-compat.h | 31 +++++++++++++++++++++++++++++++
 fs/compat_binfmt_elf.c                |  8 --------
 include/linux/elfcore-compat.h        | 18 +++++++++++-------
 6 files changed, 46 insertions(+), 29 deletions(-)
 create mode 100644 arch/x86/include/asm/elfcore-compat.h

index 56b6ccc0e32d..b4d7eb7ef833 100644
@@ -1028,6 +1028,9 @@ config HAVE_STATIC_CALL_INLINE
 	bool
 	depends on HAVE_STATIC_CALL
 
+config ARCH_HAS_ELFCORE_COMPAT
+	bool
+
 source "kernel/gcov/Kconfig"
 
 source "scripts/gcc-plugins/Kconfig"

index f6946b81f74a..8eea77cf53a7 100644
@@ -30,6 +30,7 @@ config X86_64
 	select MODULES_USE_ELF_RELA
 	select NEED_DMA_MAP_STATE
 	select SWIOTLB
+	select ARCH_HAS_ELFCORE_COMPAT
 
 config FORCE_DYNAMIC_FTRACE
 	def_bool y

index 1897e1dcbdd2..423bafe8b797 100644
@@ -159,20 +159,6 @@ struct compat_shmid64_ds {
 	compat_ulong_t __unused5;
 };
 
-/*
- * The type of struct elf_prstatus.pr_reg in compatible core dumps.
- */
-typedef struct user_regs_struct compat_elf_gregset_t;
-
-/* Full regset -- prstatus on x32, otherwise on ia32 */
-#define COMPAT_PRSTATUS_SIZE (test_thread_flag(TIF_X32) \
-	? sizeof(struct compat_elf_prstatus) \
-	: 144)
-#define COMPAT_SET_PR_FPVALID(S) \
-	(*(test_thread_flag(TIF_X32)	\
-	       ? &(S)->pr_fpvalid	\
-               : (int *)((void *)(S) + 140)) = 1)
-
 #ifdef CONFIG_X86_X32_ABI
 #define COMPAT_USE_64BIT_TIME \
 	(!!(task_pt_regs(current)->orig_ax & __X32_SYSCALL_BIT))

new file mode 100644
index 000000000000..614308ba3b95
@@ -0,0 +1,31 @@
+#ifndef _ASM_X86_ELFCORE_COMPAT_H
+#define _ASM_X86_ELFCORE_COMPAT_H
+
+#include <asm/user32.h>
+
+/*
+ * On amd64 we have two 32bit ABIs - i386 and x32.  The latter
+ * has bigger registers, so we use it for compat_elf_regset_t.
+ * The former uses i386_elf_prstatus and PRSTATUS_SIZE/SET_PR_FPVALID
+ * are used to choose the size and location of ->pr_fpvalid of
+ * the layout actually used.
+ */
+typedef struct user_regs_struct compat_elf_gregset_t;
+
+struct i386_elf_prstatus
+{
+	struct compat_elf_prstatus_common	common;
+	struct user_regs_struct32		pr_reg;
+	compat_int_t			pr_fpvalid;
+};
+
+#define PRSTATUS_SIZE \
+	(test_thread_flag(TIF_X32) \
+		? sizeof(struct compat_elf_prstatus) \
+		: sizeof(struct i386_elf_prstatus))
+#define SET_PR_FPVALID(S) \
+	(*(test_thread_flag(TIF_X32) \
+		? &(S)->pr_fpvalid 	\
+		: &((struct i386_elf_prstatus *)(S))->pr_fpvalid) = 1)
+
+#endif

index a40df32acff5..41b51b99f492 100644
@@ -96,14 +96,6 @@
 #define	ELF_EXEC_PAGESIZE	COMPAT_ELF_EXEC_PAGESIZE
 #endif
 
-#ifdef	COMPAT_PRSTATUS_SIZE
-#define	PRSTATUS_SIZE COMPAT_PRSTATUS_SIZE
-#endif
-
-#ifdef	COMPAT_SET_PR_FPVALID
-#define	SET_PR_FPVALID(S) COMPAT_SET_PR_FPVALID(S)
-#endif
-
 #ifdef	COMPAT_ELF_PLAT_INIT
 #undef	ELF_PLAT_INIT
 #define	ELF_PLAT_INIT		COMPAT_ELF_PLAT_INIT

index 4aeda5f1f038..e272c3d452ce 100644
@@ -33,13 +33,6 @@ struct compat_elf_prstatus_common
 	struct old_timeval32		pr_cstime;
 };
 
-struct compat_elf_prstatus
-{
-	struct compat_elf_prstatus_common	common;
-	compat_elf_gregset_t		pr_reg;
-	compat_int_t			pr_fpvalid;
-};
-
 struct compat_elf_prpsinfo
 {
 	char				pr_state;

@@ -54,4 +47,15 @@ struct compat_elf_prpsinfo
 	char				pr_psargs[ELF_PRARGSZ];
 };
 
+#ifdef CONFIG_ARCH_HAS_ELFCORE_COMPAT
+#include <asm/elfcore-compat.h>
+#endif
+
+struct compat_elf_prstatus
+{
+	struct compat_elf_prstatus_common	common;
+	compat_elf_gregset_t		pr_reg;
+	compat_int_t			pr_fpvalid;
+};
+
 #endif /* _LINUX_ELFCORE_COMPAT_H */

2.11.0

From: Al Viro <[email protected]>

... rather than duplicating declarations from it.

---
 arch/mips/kernel/binfmt_elfn32.c | 37 ++++---------------------------------
 arch/mips/kernel/binfmt_elfo32.c | 36 ++++--------------------------------
 2 files changed, 8 insertions(+), 65 deletions(-)

index 136dc0c9300d..b78821a2b216 100644
@@ -42,46 +42,17 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
 #include <linux/elfcore.h>
 #include <linux/compat.h>
 #include <linux/math64.h>
+#include <linux/elfcore-compat.h>
 
 #define elf_prstatus elf_prstatus32
-#define elf_prstatus_common elf_prstatus32_common
-struct elf_prstatus32_common
-{
-	struct elf_siginfo pr_info;	/* Info associated with signal */
-	short	pr_cursig;		/* Current signal */
-	unsigned int pr_sigpend;	/* Set of pending signals */
-	unsigned int pr_sighold;	/* Set of held signals */
-	pid_t	pr_pid;
-	pid_t	pr_ppid;
-	pid_t	pr_pgrp;
-	pid_t	pr_sid;
-	struct old_timeval32 pr_utime; /* User time */
-	struct old_timeval32 pr_stime; /* System time */
-	struct old_timeval32 pr_cutime;/* Cumulative user time */
-	struct old_timeval32 pr_cstime;/* Cumulative system time */
-};
+#define elf_prstatus_common compat_elf_prstatus_common
 struct elf_prstatus32
 {
-	struct elf_prstatus32_common common:
+	struct compat_elf_prstatus_common common;
 	elf_gregset_t pr_reg;	/* GP registers */
 	int pr_fpvalid;		/* True if math co-processor being used.  */
 };
-
-#define elf_prpsinfo elf_prpsinfo32
-struct elf_prpsinfo32
-{
-	char	pr_state;	/* numeric process state */
-	char	pr_sname;	/* char for pr_state */
-	char	pr_zomb;	/* zombie */
-	char	pr_nice;	/* nice val */
-	unsigned int pr_flag;	/* flags */
-	__kernel_uid_t	pr_uid;
-	__kernel_gid_t	pr_gid;
-	pid_t	pr_pid, pr_ppid, pr_pgrp, pr_sid;
-	/* Lots missing */
-	char	pr_fname[16];	/* filename of executable */
-	char	pr_psargs[ELF_PRARGSZ]; /* initial part of arg list */
-};
+#define elf_prpsinfo compat_elf_prpsinfo
 
 #define elf_caddr_t	u32
 #define init_elf_binfmt init_elfn32_binfmt

index b1f4b8f1dee7..e94afbd3adc5 100644
@@ -47,45 +47,17 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
 #include <linux/elfcore.h>
 #include <linux/compat.h>
 #include <linux/math64.h>
+#include <linux/elfcore-compat.h>
 
 #define elf_prstatus elf_prstatus32
-struct elf_prstatus32_common
-{
-	struct elf_siginfo pr_info;	/* Info associated with signal */
-	short	pr_cursig;		/* Current signal */
-	unsigned int pr_sigpend;	/* Set of pending signals */
-	unsigned int pr_sighold;	/* Set of held signals */
-	pid_t	pr_pid;
-	pid_t	pr_ppid;
-	pid_t	pr_pgrp;
-	pid_t	pr_sid;
-	struct old_timeval32 pr_utime; /* User time */
-	struct old_timeval32 pr_stime; /* System time */
-	struct old_timeval32 pr_cutime;/* Cumulative user time */
-	struct old_timeval32 pr_cstime;/* Cumulative system time */
-};
+#define elf_prstatus_common compat_elf_prstatus_common
 struct elf_prstatus32
 {
-	struct elf_prstatus32_common common:
+	struct compat_elf_prstatus_common common;
 	elf_gregset_t pr_reg;	/* GP registers */
 	int pr_fpvalid;		/* True if math co-processor being used.  */
 };
-
-#define elf_prpsinfo elf_prpsinfo32
-struct elf_prpsinfo32
-{
-	char	pr_state;	/* numeric process state */
-	char	pr_sname;	/* char for pr_state */
-	char	pr_zomb;	/* zombie */
-	char	pr_nice;	/* nice val */
-	unsigned int pr_flag;	/* flags */
-	__kernel_uid_t	pr_uid;
-	__kernel_gid_t	pr_gid;
-	pid_t	pr_pid, pr_ppid, pr_pgrp, pr_sid;
-	/* Lots missing */
-	char	pr_fname[16];	/* filename of executable */
-	char	pr_psargs[ELF_PRARGSZ]; /* initial part of arg list */
-};
+#define elf_prpsinfo compat_elf_prpsinfo
 
 #define elf_caddr_t	u32
 #define init_elf_binfmt init_elf32_binfmt

2.11.0

From: Al Viro <[email protected]>

elf_caddr_t: unused since 2002
jiffies_to_timeval: unused since 2015
TASK_SIZE: used only downstream of SET_PERSONALITY2(), and after that point the normal definition results in TASK_SIZE32 just fine.

---
 arch/mips/kernel/binfmt_elfn32.c | 18 ------------------
 arch/mips/kernel/binfmt_elfo32.c | 18 ------------------
 2 files changed, 36 deletions(-)

index b78821a2b216..4f8bfaf414f5 100644
@@ -54,28 +54,10 @@ struct elf_prstatus32
 };
 #define elf_prpsinfo compat_elf_prpsinfo
 
-#define elf_caddr_t	u32
 #define init_elf_binfmt init_elfn32_binfmt
 
-#define jiffies_to_timeval jiffies_to_old_timeval32
-static __inline__ void
-jiffies_to_old_timeval32(unsigned long jiffies, struct old_timeval32 *value)
-{
-	/*
-	 * Convert jiffies to nanoseconds and separate with
-	 * one divide.
-	 */
-	u64 nsec = (u64)jiffies * TICK_NSEC;
-	u32 rem;
-	value->tv_sec = div_u64_rem(nsec, NSEC_PER_SEC, &rem);
-	value->tv_usec = rem / NSEC_PER_USEC;
-}
-
 #define ELF_CORE_EFLAGS EF_MIPS_ABI2
 
-#undef TASK_SIZE
-#define TASK_SIZE TASK_SIZE32
-
 #undef ns_to_kernel_old_timeval
 #define ns_to_kernel_old_timeval ns_to_old_timeval32
 

index e94afbd3adc5..9755ff12a9fe 100644
@@ -59,26 +59,8 @@ struct elf_prstatus32
 };
 #define elf_prpsinfo compat_elf_prpsinfo
 
-#define elf_caddr_t	u32
 #define init_elf_binfmt init_elf32_binfmt
 
-#define jiffies_to_timeval jiffies_to_old_timeval32
-static inline void
-jiffies_to_old_timeval32(unsigned long jiffies, struct old_timeval32 *value)
-{
-	/*
-	 * Convert jiffies to nanoseconds and separate with
-	 * one divide.
-	 */
-	u64 nsec = (u64)jiffies * TICK_NSEC;
-	u32 rem;
-	value->tv_sec = div_u64_rem(nsec, NSEC_PER_SEC, &rem);
-	value->tv_usec = rem / NSEC_PER_USEC;
-}
-
-#undef TASK_SIZE
-#define TASK_SIZE TASK_SIZE32
-
 #undef ns_to_kernel_old_timeval
 #define ns_to_kernel_old_timeval ns_to_old_timeval32
 

2.11.0

From: Al Viro <[email protected]>

it's always been about MIPS32

---
 arch/mips/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 2000bb2b0220..32ce052aa3b4 100644
@@ -2179,7 +2179,7 @@ endchoice
 config KVM_GUEST
 	bool "KVM Guest Kernel"
 	depends on CPU_MIPS32_R2
-	depends on BROKEN_ON_SMP
+	depends on !64BIT && BROKEN_ON_SMP
 	help
 	  Select this option if building a guest kernel for KVM (Trap & Emulate)
 	  mode.

2.11.0

From: Al Viro <[email protected]>

normal mips one is just fine - it's only used after we'd done SET_PERSONALITY2() and by that point TASK_SIZE will yield the right value

---
 arch/mips/include/asm/elf.h      | 2 --
 arch/mips/kernel/binfmt_elfn32.c | 4 ----
 arch/mips/kernel/binfmt_elfo32.c | 8 --------
 3 files changed, 14 deletions(-)

index 71c7622025d1..d29e43e4f9b1 100644
@@ -469,9 +469,7 @@ extern const char *__elf_base_platform;
    the loader.	We need to make sure that it is out of the way of the program
    that it will "exec", and that there is sufficient room for the brk.	*/
 
-#ifndef ELF_ET_DYN_BASE
 #define ELF_ET_DYN_BASE		(TASK_SIZE / 3 * 2)
-#endif
 
 /* update AT_VECTOR_SIZE_ARCH if the number of NEW_AUX_ENT entries changes */
 #define ARCH_DLINFO							\

index 4f8bfaf414f5..10fa9ccd92b7 100644
@@ -34,10 +34,6 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
  */
 #define elf_check_arch elfn32_check_arch
 
-#define TASK32_SIZE		0x7fff8000UL
-#undef ELF_ET_DYN_BASE
-#define ELF_ET_DYN_BASE		(TASK32_SIZE / 3 * 2)
-
 #include <asm/processor.h>
 #include <linux/elfcore.h>
 #include <linux/compat.h>

index 9755ff12a9fe..54fe6a4dac6d 100644
@@ -34,14 +34,6 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
  */
 #define elf_check_arch elfo32_check_arch
 
-#ifdef CONFIG_KVM_GUEST
-#define TASK32_SIZE		0x3fff8000UL
-#else
-#define TASK32_SIZE		0x7fff8000UL
-#endif
-#undef ELF_ET_DYN_BASE
-#define ELF_ET_DYN_BASE		(TASK32_SIZE / 3 * 2)
-
 #include <asm/processor.h>
 
 #include <linux/elfcore.h>

2.11.0

From: Al Viro <[email protected]>

mips coredumps are regset-based, so ELF_CORE_EFLAGS is not used at all - user_..._view.e_flags is.

---
 arch/mips/kernel/binfmt_elfn32.c | 2 --
 1 file changed, 2 deletions(-)

index 10fa9ccd92b7..1aa32570fa9b 100644
@@ -52,8 +52,6 @@ struct elf_prstatus32
 
 #define init_elf_binfmt init_elfn32_binfmt
 
-#define ELF_CORE_EFLAGS EF_MIPS_ABI2
-
 #undef ns_to_kernel_old_timeval
 #define ns_to_kernel_old_timeval ns_to_old_timeval32
 

2.11.0

From: Al Viro <[email protected]>

Like amd64, mips has two 32bit ABIs - o32 and n32. Unlike amd64, it does not use compat_binfmt_elf.c for either of those; each of those ABIs has a binfmt handler of its own, both very similar to fs/compat_binfmt_elf.c. And the same technics as we use on amd64 can be used to make fs/compat_binfmt_elf.c handle both.
* merge elfo32_check_arch() with elfn32_check_arch(),
make that serve as compat_elf_check_arch(). Note that SET_PERSONALITY2() is already the same for all ABI variants - it looks at the elf header to choose the flags to set.
* add asm/elfcore-compat.h, using the bigger (n32) variant
of elf32_prstatus as compat_elf_prstatus there.
* make PRSTATUS_SIZE() and SET_PR_FPVALID() choose the
right layout, same as done for amd64. test_thread_flag(TIF_32BIT_REGS) is used as the predicate.

Voila - we are rid of binfmt_elf{n,o}32.c; fs/compat_binfmt_elf.c is used, same as for all other ELF-supporting 64bit architectures that need 32bit compat.

---
 arch/mips/Kconfig                      |  8 ++---
 arch/mips/include/asm/elf.h            | 54 +++++++++++--------------------
 arch/mips/include/asm/elfcore-compat.h | 29 +++++++++++++++++
 arch/mips/kernel/Makefile              |  4 +--
 arch/mips/kernel/binfmt_elfn32.c       | 58 ---------------------------------
 arch/mips/kernel/binfmt_elfo32.c       | 59 ----------------------------------
 arch/mips/kernel/scall64-n64.S         |  2 +-
 7 files changed, 54 insertions(+), 160 deletions(-)
 create mode 100644 arch/mips/include/asm/elfcore-compat.h
 delete mode 100644 arch/mips/kernel/binfmt_elfn32.c
 delete mode 100644 arch/mips/kernel/binfmt_elfo32.c

index 32ce052aa3b4..f14731aee182 100644
@@ -91,6 +91,7 @@ config MIPS
 	select SET_FS
 	select SYSCTL_EXCEPTION_TRACE
 	select VIRT_TO_BUS
+	select ARCH_HAS_ELFCORE_COMPAT
 
 config MIPS_FIXUP_BIGPHYS_ADDR
 	bool

@@ -3270,6 +3271,7 @@ config MIPS32_O32
 	select ARCH_WANT_OLD_COMPAT_IPC
 	select COMPAT
 	select MIPS32_COMPAT
+	select COMPAT_BINFMT_ELF
 	select SYSVIPC_COMPAT if SYSVIPC
 	help
 	  Select this option if you want to run o32 binaries.  These are pure

@@ -3283,6 +3285,7 @@ config MIPS32_N32
 	depends on 64BIT
 	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
 	select COMPAT
+	select COMPAT_BINFMT_ELF
 	select MIPS32_COMPAT
 	select SYSVIPC_COMPAT if SYSVIPC
 	help

@@ -3293,11 +3296,6 @@ config MIPS32_N32
 
 	  If unsure, say N.
 
-config BINFMT_ELF32
-	bool
-	default y if MIPS32_O32 || MIPS32_N32
-	select ELFCORE
-
 menu "Power management options"
 
 config ARCH_HIBERNATION_POSSIBLE

new file mode 100644
index 000000000000..2f0f0103c75b
@@ -0,0 +1,29 @@
+#ifndef _ASM_MIPS_ELFCORE_COMPAT_H
+#define _ASM_MIPS_ELFCORE_COMPAT_H
+
+/*
+ * On mips we have two 32bit ABIs - o32 and n32.  The latter
+ * has bigger registers, so we use it for compat_elf_regset_t.
+ * The former uses o32_elf_prstatus and PRSTATUS_SIZE/SET_PR_FPVALID
+ * are used to choose the size and location of ->pr_fpvalid of
+ * the layout actually used.
+ */
+typedef elf_gregset_t compat_elf_gregset_t;
+
+struct o32_elf_prstatus
+{
+	struct compat_elf_prstatus_common	common;
+	unsigned int 			pr_reg[ELF_NGREG];
+	compat_int_t			pr_fpvalid;
+};
+
+#define PRSTATUS_SIZE \
+	(!test_thread_flag(TIF_32BIT_REGS) \
+		? sizeof(struct compat_elf_prstatus) \
+		: sizeof(struct o32_elf_prstatus))
+#define SET_PR_FPVALID(S) \
+	(*(!test_thread_flag(TIF_32BIT_REGS) \
+		? &(S)->pr_fpvalid 	\
+		: &((struct o32_elf_prstatus *)(S))->pr_fpvalid) = 1)
+
+#endif

index 2a05b923f579..943eaeef73e9 100644
@@ -80,8 +80,8 @@ obj-$(CONFIG_KPROBES)		+= kprobes.o
 obj-$(CONFIG_32BIT)		+= scall32-o32.o
 obj-$(CONFIG_64BIT)		+= scall64-n64.o
 obj-$(CONFIG_MIPS32_COMPAT)	+= linux32.o ptrace32.o signal32.o
-obj-$(CONFIG_MIPS32_N32)	+= binfmt_elfn32.o scall64-n32.o signal_n32.o
-obj-$(CONFIG_MIPS32_O32)	+= binfmt_elfo32.o scall64-o32.o signal_o32.o
+obj-$(CONFIG_MIPS32_N32)	+= scall64-n32.o signal_n32.o
+obj-$(CONFIG_MIPS32_O32)	+= scall64-o32.o signal_o32.o
 
 obj-$(CONFIG_KGDB)		+= kgdb.o
 obj-$(CONFIG_PROC_FS)		+= proc.o

index 23b2e2b1609c..5e9c497ce099 100644
@@ -20,7 +20,7 @@
 #include <asm/unistd.h>
 #include <asm/war.h>
 
-#ifndef CONFIG_BINFMT_ELF32
+#ifndef CONFIG_MIPS32_COMPAT
 /* Neither O32 nor N32, so define handle_sys here */
 #define handle_sys64 handle_sys
 #endif

2.11.0

From: Al Viro <[email protected]>

with mips converted to use of fs/config_binfmt_elf.c, there's no need to keep selects of that thing all over arch/* - we can simply turn into def_bool y if COMPAT && BINFMT_ELF (in fs/Kconfig.binfmt) and get rid of all selects.

Several architectures got those selects wrong (e.g. you could end up with sparc64 sans BINFMT_ELF, with select violating dependencies, or with amd64 with X32 and BINFMT_ELF enabled, but unable to exec any X32 binaries), etc.

Randy Dunlap has spotted some of those; IMO this is simpler than his fix, but it depends upon the stuff that would need to be backported, so we might end up using his variant for -stable.

---
 arch/arm64/Kconfig   | 1 -
 arch/mips/Kconfig    | 2 --
 arch/parisc/Kconfig  | 1 -
 arch/powerpc/Kconfig | 1 -
 arch/s390/Kconfig    | 1 -
 arch/sparc/Kconfig   | 1 -
 arch/x86/Kconfig     | 1 -
 fs/Kconfig.binfmt    | 2 +-
 8 files changed, 1 insertion(+), 9 deletions(-)

index f858c352f72a..84a0af4ba8d7 100644
@@ -1196,7 +1196,6 @@ config ARM64_TAGGED_ADDR_ABI
 menuconfig COMPAT
 	bool "Kernel support for 32-bit EL0"
 	depends on ARM64_4K_PAGES || EXPERT
-	select COMPAT_BINFMT_ELF if BINFMT_ELF
 	select HAVE_UID16
 	select OLD_SIGSUSPEND3
 	select COMPAT_OLD_SIGACTION

index f14731aee182..df35ce61aa81 100644
@@ -3271,7 +3271,6 @@ config MIPS32_O32
 	select ARCH_WANT_OLD_COMPAT_IPC
 	select COMPAT
 	select MIPS32_COMPAT
-	select COMPAT_BINFMT_ELF
 	select SYSVIPC_COMPAT if SYSVIPC
 	help
 	  Select this option if you want to run o32 binaries.  These are pure

@@ -3285,7 +3284,6 @@ config MIPS32_N32
 	depends on 64BIT
 	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
 	select COMPAT
-	select COMPAT_BINFMT_ELF
 	select MIPS32_COMPAT
 	select SYSVIPC_COMPAT if SYSVIPC
 	help

index b234e8154cbd..97c233a7445c 100644
@@ -336,7 +336,6 @@ source "kernel/Kconfig.hz"
 config COMPAT
 	def_bool y
 	depends on 64BIT
-	select COMPAT_BINFMT_ELF if BINFMT_ELF
 
 config SYSVIPC_COMPAT
 	def_bool y

index e9f13fe08492..d27469a024a5 100644
@@ -278,7 +278,6 @@ config COMPAT
 	bool "Enable support for 32bit binaries"
 	depends on PPC64
 	default y if !CPU_LITTLE_ENDIAN
-	select COMPAT_BINFMT_ELF
 	select ARCH_WANT_OLD_COMPAT_IPC
 	select COMPAT_OLD_SIGACTION
 

index 4a2a12be04c9..55d140044f32 100644
@@ -426,7 +426,6 @@ config 64BIT
 config COMPAT
 	def_bool y
 	prompt "Kernel support for 31 bit emulation"
-	select COMPAT_BINFMT_ELF if BINFMT_ELF
 	select ARCH_WANT_OLD_COMPAT_IPC
 	select COMPAT_OLD_SIGACTION
 	select HAVE_UID16

index a6ca135442f9..12943d94fcd0 100644
@@ -496,7 +496,6 @@ config COMPAT
 	bool
 	depends on SPARC64
 	default y
-	select COMPAT_BINFMT_ELF
 	select HAVE_UID16
 	select ARCH_WANT_OLD_COMPAT_IPC
 	select COMPAT_OLD_SIGACTION

index 8eea77cf53a7..638fdf3e6578 100644
@@ -2844,7 +2844,6 @@ config IA32_EMULATION
 	depends on X86_64
 	select ARCH_WANT_OLD_COMPAT_IPC
 	select BINFMT_ELF
-	select COMPAT_BINFMT_ELF
 	select COMPAT_OLD_SIGACTION
 	help
 	  Include code to run legacy 32-bit programs under a

index 885da6d983b4..b32f5df68ae9 100644
@@ -29,7 +29,7 @@ config BINFMT_ELF
 	  latest version).
 
 config COMPAT_BINFMT_ELF
-	bool
+	def_bool y
 	depends on COMPAT && BINFMT_ELF
 	select ELFCORE
 

2.11.0

Well, the diffstat looks nice:

and the patches didn't trigger anything for me, but how much did this get tested? Do you actually have both kinds of 32-bit elf mips binaries around and a machine to test on?

Linux-mips was cc'd, but I'm adding Thomas B to the cc here explicitly just so that he has a heads-up on this thing and can go and look at the mailing list in case it goes to a separate mailbox for him..

Linus

Yes (aptitude install gcc-multilib on debian mips64el/stretch sets the toolchain and libraries just fine, and then it's just a matter of -mabi=n32 passed to gcc). "Machine" is qemu-system-mips64el -machine malta -m 1024 -cpu 5KEc and the things appear to work; I hadn't tried that on the actual hardware. I do have a Loongson-2 box, but it would take a while to dig it out and get it up-to-date.

I would certainly appreciate review and testing - this branch sat around in the "should post it someday" state since June (it was one of the followups grown from regset work back then), and I'm _not_ going to ask pulling it without an explicit OK from mips folks.

BTW, there's something curious going on in ELF binary recognition for x32. Unlike other 64bit architectures, here we have a 32bit binary that successfully passes the native elf_check_arch(). Usually we either have different EM_... values for 64bit and 32bit (e.g. for ppc and sparc) or we have an explicit check for ->e_ident[EI_CLASS] having the right value (ELFCLASS32 or ELFCLASS64 for 32bit and 64bit binaries resp.)

For x32 that's not true - we use EM_X86_64 for ->e_machine and that's the only thing the native elf_check_arch() is looking at. IOW, it looks like amd64 elf_load_binary() progresses past elf_check_arch() for x32 binaries. And gets to load_elf_phdrs(), which would appear to have a check of its own that should reject the sucker:

        /*
         * If the size of this structure has changed, then punt, since
         * we will be doing the wrong thing.
         */
        if (elf_ex->e_phentsize != sizeof(struct elf_phdr))
                goto out;

After all, ->e_phentsize is going to be 32 (sizeof(struct elf32_phdr) rather than expected 56 (sizeof(struct elf64_phdr)) and off we bugger, even though it happens at slightly later point than usual. Except that we are looking at struct elf64_hdr ->e_phentsize - in struct elf32_hdr. I.e. at offset 54, two bytes past the end of in-file struct elf32_hdr.

Usually we won't find 0x38 0x00 in that location, so everything works, but IMO that's too convoluted.

Peter, is there any reason not to check ->ei_ident[EI_CLASS] in amd64 elf_check_arch()? It's a 1-byte load from hot cacheline (offset 4 and we'd just read the 4 bytes at offsets 0..3) + compare + branch not taken, so performance impact is pretty much nil. I'm not saying it's a security problem or anything of that sort, just that it makes the analysis more subtle than it ought to be...

Is it about some malformed homegrown 64bit binaries with BS value at offset 4? Confused...

I can't think of any.

Sent from my Android device with K-9 Mail. Please excuse my brevity.

It may be worth pushing through GDB's gdb.threads/tls-core.exp test case,
making sure no UNSUPPORTED results have been produced due to resource limits preventing a core from being dumped (and no FAILs, of course), with o32/n32 native GDB. This should guarantee our output is still as expected by an interpreter. Sadly I'm currently not set up for such testing though eventually I mean to.

Maciej

Umm... What triple does one use for n32 gdb?

I don't think there's a standardised one, just configure with CC/CXX set
for n32 compilation, e.g.:

$ /path/to/configure CC="gcc -mabi=n32" CXX="g++ -mabi=n32"

(and any other options set as usually). This has to be with CC/CXX rather than CFLAGS/CXXFLAGS so that it is guaranteed to be never overridden with any logic that might do any fiddling with compilation options. This will set up the test suite accordingly.

NB this may already be the compiler's default, depending on how it was
configured, i.e. if --with-abi=n32' was used, in which case no extra options will be required. I don't know if any standard MIPS distribution does it though; 64-bit MIPS/Debian might. This will be reported with gcc --help -v', somewhere along the way.

Let me know if there are issues with this approach.

Maciej

FWIW, on debian/mips64el (both stretch and buster) the test fails with the distro kernels (4.9- and 4.19-based) as well as with 5.10-rc1 and 5.10-rc1+that series, all in the same way:
[Current thread is 1 (LWP 4154)]
(gdb) p/x foo
Cannot find thread-local storage for LWP 4154, executable file <pathname> Cannot find thread-local variables on this target

buster has libc6-2.28, so that should be fine for the test in question (libthread_db definitely recent enough). That was n32 gdb; considering how much time it had taken to build that sucker I hadn't tried o32 yet.

Note that it's not just with native coredumps - gcore-produced ones give the same result. That was gdb from binutils-gdb.git; I'm not familiar with gdb guts to start debugging it, so if you have any suggestions in that direction that do not include a full rebuild... In any case, I won't get around to that until the next week.

Incidentally, build time is bloody awful - 3 days, with qemu-3.1 on 3.5GHz amd64 host, all spent pretty much entirely in userland (both from guest and host POV). g++-8 is atrociously slow...

That said, I don't see what in that series could possibly mess the things up for tls, while leaving the registers working; the only thing that realistically might've been fucked up is prstatus layout (and possibly size), and that would've screwed the registers as well.

... and it smells like the damn thing needs n32 debug info from libthread_db.so and/or libpthread.so. Which is not packaged by debian libc6 mips64el build. Sorry, any debugging of that crap is going to happen in January ;-/

Cute... Completely unrelated, but there's a fun bug in mainline o32 coredumps - say readelf -a core and watch NT_FILE section dump. Compare that for dumps done on mips32 and mips64 hosts (for the same o32 binaries, obviously). Or to gcore(1) results on such processes, for that matter.

What happens there is that 2aa362c49c31 ("coredump: extend core dump note section to contain file names of mapped files") that has introduced that section has added
#define user_long_t compat_long_t
to fs/compat_binfmt_elf.c, but not to arch/mips/kernel/binfmt_elfo32.c, resulting in default (long) being used by fill_files_note().

[Denys Vlasenko cc'd]

One issue is that testsuite doesn't care about $CC, $CFLAGS or anything of that sort. What I'd done was
cat >~/bin/cc-n32 <<'EOF'
#!/bin/sh
exec /usr/bin/gcc -mabi=n32 "$@"
EOF
chmod +x ~/bin/cc-n32
and add CC_FOR_TARGET="/home/al/bin/cc-n32" in RUNTESTFLAGS.

With that it works. Moreover, it fixes a test failure on mainline. Mainline kernel (5.10, same behaviour as debian/buster mips64el one):
Test run by al on Tue Dec 22 21:23:09 2020
Native configuration is mips64el-unknown-linux-gnuabin32

                === gdb tests ===

Schedule of variations:
unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target. Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/al/binutils-gdb/gdb/testsuite/config/unix.exp as tool-and-target-specific interface file. Running /home/al/binutils-gdb/gdb/testsuite/gdb.threads/tls-core.exp ...
FAIL: gdb.threads/tls-core.exp: native: print thread-local storage variable

                === gdb Summary ===

# of expected passes            5
# of unexpected failures        1

vfs.git #work.elf-compat:
Test run by al on Tue Dec 22 21:31:14 2020 Native configuration is mips64el-unknown-linux-gnuabin32

                === gdb tests ===

Schedule of variations:
unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target. Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/al/binutils-gdb/gdb/testsuite/config/unix.exp as tool-and-target-specific interface file. Running /home/al/binutils-gdb/gdb/testsuite/gdb.threads/tls-core.exp ...

                === gdb Summary ===

# of expected passes            6

Which is bloody embarrassing, since I'd completely missed the behaviour change - this series was supposed to be an equivalent transformation.

Anyway, the minimal patch fixing that failure is this one-liner and unlike the elf-compat series it's trivial to backport:

[mips] fix n32 coredump breakage

Back in 2012, 49ae4d4b113b ("coredump: add a new elf note with siginfo of the signal") has introduced a new ELF coredump note - NT_FILE. It contains a mix of strings and addresses, and addresses are 32bit for 32bit targets and 64bit for 64bit ones. Eventually gdb has come to use it.

Biarch targets had been taken care of from the very beginning - the same commit has added a macro (user_long_t) with default being long and fs/compat_binfmt_elf.c overriding it to compat_long_t.

Unfortunately, Denis had missed the mips weirdness. As the result, on mips64 both o32 and n32 ended up using 64-bit layout. readelf(1) is not happy. More importantly, neither is gdb(1); as the matter of fact, gdb.thread/tls-core.exp kept complaining. Note that gcore(1) is using 32bit layout for n32 case - it's only the kernel n32 coredumps that get broken NT_FILE note.

NOTE: similar patch is almost certainly needed for o32; I have only tested it with n32 gdb, though.

---

index 6ee3f7218c67..c073136968e8 100644
@@ -103,4 +103,6 @@ jiffies_to_old_timeval32(unsigned long jiffies, struct old_timeval32 *value)
 #undef ns_to_kernel_old_timeval
 #define ns_to_kernel_old_timeval ns_to_old_timeval32
 
+#define user_long_t compat_long_t
+
 #include "../../../fs/binfmt_elf.c"

On Wed, Dec 23, 2020 at 07:03:20AM +0000, Al Viro wrote:

Argh.... Wrong commit blamed - the parent of the correct one.
It's actually 2aa362c49c31 ("coredump: extend core dump note section to contain file names of mapped files"). My apologies - fat-fingered cut'n'paste...

siginfo commit does suffer the same problem, but it becomes an issue
only for 32bit processes under mips64 big-endian kernel (there it yields e.g. zero .__sigfault.si_addr in $_siginfo when using gdb with a coredump of 32bit process, whatever the actual faulting address had been). And b-e mips64 is rather uncommon, so that's less of an issue.

FWIW, here's debian/mips image (stretch) booted with
qemu-system-mips64 -M malta -cpu 5KEc:

root@mips:~# uname -a
Linux mips 4.9.0-13-5kc-malta #1 Debian 4.9.228-1 (2020-07-05) mips64 GNU/Linux
root@mips:~# cat a.c
main()
{
        *(char *)0x0123 = 0;
}
root@mips:~# gcc a.c 
a.c:1:1: warning: return type defaults to ‘int’ [-Wimplicit-int]
 main()
 ^~~~
root@mips:~# ulimit -c unlimited
root@mips:~# ./a.out 
[  519.744983] do_page_fault(): sending SIGSEGV to a.out for invalid write access to 0000000000000123
[  519.746735] epc = 00000000558477c0 in a.out[55847000+1000]
[  519.747758] ra  = 000000007792f4a8 in libc-2.24.so[77916000+16a000]
Segmentation fault (core dumped)
root@mips:~# gdb a.out core 
GNU gdb (Debian 7.12-6) 7.12.0.20161007-git
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "mips-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from a.out...(no debugging symbols found)...done.
[New LWP 1202]
Core was generated by `./a.out'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x55dde7c0 in main ()
(gdb) print $_siginfo
$1 = {si_signo = 11, si_errno = 1, si_code = 0, _sifields = {_pad = {0, 0, 
      291, 0 <repeats 26 times>}, _kill = {si_pid = 0, si_uid = 0}, _timer = {
      si_tid = 0, si_overrun = 0, si_sigval = {sival_int = 291, 
        sival_ptr = 0x123}}, _rt = {si_pid = 0, si_uid = 0, si_sigval = {
        sival_int = 291, sival_ptr = 0x123}}, _sigchld = {si_pid = 0, 
      si_uid = 0, si_status = 291, si_utime = 0, si_stime = 0}, _sigfault = {
      si_addr = 0x0}, _sigpoll = {si_band = 0, si_fd = 0}}}
(gdb) quit

Note the wrong value in _sigfault.si_addr - it should've been 0x123, not 0.

root@mips:~# readelf -n core

Displaying notes found at file offset 0x00000234 with length 0x000005f4:

  Owner                 Data size       Description
  CORE                 0x00000100       NT_PRSTATUS (prstatus structure)
  CORE                 0x00000080       NT_PRPSINFO (prpsinfo structure)
  CORE                 0x00000080       NT_SIGINFO (siginfo_t data)
  CORE                 0x00000090       NT_AUXV (auxiliary vector)
  CORE                 0x000001e1       NT_FILE (mapped files)

Page size: 9
Start End Page Offset
CORE 0x00000108 NT_FPREGSET (floating point registers)

For comparison, exact same image booted with qemu-system-mips -M malta:

root@mips:~# uname -a
Linux mips 4.9.0-13-4kc-malta #1 Debian 4.9.228-1 (2020-07-05) mips GNU/Linux
root@mips:~# ulimit -c unlimited
root@mips:~# ./a.out
[   83.380870] do_page_fault(): sending SIGSEGV to a.out for invalid write access to 00000123
[   83.390678] epc = 55e0e7c0 in a.out[55e0e000+1000]
[   83.391525] ra  = 76f644a8 in libc-2.24.so[76f4b000+16a000]
Segmentation fault (core dumped)
root@mips:~# gdb a.out core
GNU gdb (Debian 7.12-6) 7.12.0.20161007-git
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "mips-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from a.out...(no debugging symbols found)...done.
[New LWP 1184]
Core was generated by `./a.out'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x55e0e7c0 in main ()
(gdb) print $_siginfo
$1 = {si_signo = 11, si_errno = 1, si_code = 0, _sifields = {_pad = {291, 
      0 <repeats 28 times>}, _kill = {si_pid = 291, si_uid = 0}, _timer = {
      si_tid = 291, si_overrun = 0, si_sigval = {sival_int = 0, 
        sival_ptr = 0x0}}, _rt = {si_pid = 291, si_uid = 0, si_sigval = {
        sival_int = 0, sival_ptr = 0x0}}, _sigchld = {si_pid = 291, 
      si_uid = 0, si_status = 0, si_utime = 0, si_stime = 0}, _sigfault = {
      si_addr = 0x123}, _sigpoll = {si_band = 291, si_fd = 0}}}
(gdb) quit
root@mips:~# readelf -n core

Displaying notes found at file offset 0x00000234 with length 0x00000580:

  Owner                 Data size       Description
  CORE                 0x00000100       NT_PRSTATUS (prstatus structure)
  CORE                 0x00000080       NT_PRPSINFO (prpsinfo structure)
  CORE                 0x00000080       NT_SIGINFO (siginfo_t data)
  CORE                 0x00000090       NT_AUXV (auxiliary vector)
  CORE                 0x0000016d       NT_FILE (mapped files)
    Page size: 4096
         Start         End Page Offset
    0x55e0e000  0x55e0f000  0x00000000
        /root/a.out
    0x55e1e000  0x55e1f000  0x00000000
        /root/a.out
    0x76f4b000  0x770b5000  0x00000000
        /lib/mips-linux-gnu/libc-2.24.so
    0x770b5000  0x770c5000  0x0000016a
        /lib/mips-linux-gnu/libc-2.24.so
    0x770c5000  0x770c8000  0x0000016a
        /lib/mips-linux-gnu/libc-2.24.so
    0x770c8000  0x770cb000  0x0000016d
        /lib/mips-linux-gnu/libc-2.24.so
    0x770cd000  0x770f0000  0x00000000
        /lib/mips-linux-gnu/ld-2.24.so
    0x770ff000  0x77100000  0x00000022
        /lib/mips-linux-gnu/ld-2.24.so
    0x77100000  0x77101000  0x00000023
        /lib/mips-linux-gnu/ld-2.24.so

CORE 0x00000108 NT_FPREGSET (floating point registers)

So that's not so theoretical - big-endian mips64 userland is unsupported, but booting the big-endian mips32 userland on mips64 hardware is clearly meant to work - they even ship a 64bit kernel built for that.

IOW, both O32 and N32 coredumps in 64bit mips kernels have broken NT_FILE and NT_SIGINFO. And while NT_SIGINFO breakage is really visible only on b-e, NT_FILE one is common to b-e and l-e. One of the effects of the latter is that current gdb fails to work with threaded coredumps of 32bit processes produced on boxen with 64bit kernels. Coredumps generated by gcore(1) are fine...

I think the following ought to be applied. Comments?

[mips] fix malformed NT_FILE and NT_SIGINFO in 32bit coredumps

Patches that introduced NT_FILE and NT_SIGINFO notes back in 2012
had taken care of native (fs/binfmt_elf.c) and compat (fs/compat_binfmt_elf.c) coredumps; unfortunately, compat on mips (which does not go through the usual compat_binfmt_elf.c) had not been noticed.

As the result, both N32 and O32 coredumps on 64bit mips kernels
have those sections malformed enough to confuse the living hell out of all gdb and readelf versions (up to and including the tip of binutils-gdb.git).

Longer term solution is to make both O32 and N32 compat use the
regular compat_binfmt_elf.c, but that's too much for backports. The minimal solution is to do in arch/mips/kernel/binfmt_elf[on]32.c the same thing those patches have done in fs/compat_binfmt_elf.c

---

index 6ee3f7218c67..c4441416e96b 100644
@@ -103,4 +103,11 @@ jiffies_to_old_timeval32(unsigned long jiffies, struct old_timeval32 *value)
 #undef ns_to_kernel_old_timeval
 #define ns_to_kernel_old_timeval ns_to_old_timeval32
 
+/*
+ * Some data types as stored in coredump.
+ */
+#define user_long_t             compat_long_t
+#define user_siginfo_t          compat_siginfo_t
+#define copy_siginfo_to_external        copy_siginfo_to_external32
+
 #include "../../../fs/binfmt_elf.c"

index 6dd103d3cebb..7b2a23f48c1a 100644
@@ -106,4 +106,11 @@ jiffies_to_old_timeval32(unsigned long jiffies, struct old_timeval32 *value)
 #undef ns_to_kernel_old_timeval
 #define ns_to_kernel_old_timeval ns_to_old_timeval32
 
+/*
+ * Some data types as stored in coredump.
+ */
+#define user_long_t             compat_long_t
+#define user_siginfo_t          compat_siginfo_t
+#define copy_siginfo_to_external        copy_siginfo_to_external32
+
 #include "../../../fs/binfmt_elf.c"

I've tested it on real hardware and so far everything looks good.

You can add my

for the MIPS part.

Thomas.

Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]