linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCHES] converting FDPIC coredumps to regsets
@ 2020-06-30  4:36 Al Viro
  2020-06-30  4:41 ` [PATCH 1/7] unexport linux/elfcore.h Al Viro
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Al Viro @ 2020-06-30  4:36 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Eric W. Biederman, David Howells, Nicolas Pitre

	Conversion of ELF coredumps to regsets has not touched
ELF_FDPIC.  Right now all architectures that support FDPIC have
regsets sufficient for switching it to regset-based coredumps.  A bit
of backstory: original ELF (and ELF_FDPIC) coredumps reused the old
helpers used by a.out coredumps.  These days a.out coredumps are gone;
we could remove the dead code, if not for several obstacles.  And one
of those obstacles is ELF_FDPIC.

	This series more or less reproduces the conversion done
by Roland for ELF coredumps.  The branch is in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.fdpic
and it's based on top of #regset.base there (just the introduction of
regset_get() wrapper for ->get(); nothing else from the regset series
is needed).  Killing the old aout helpers is _not_ in this branch;
followup cleanups live separately.

	First we need to sort out the mess with struct elf_prstatus,
though.  It's used both for ELF and ELF_FDPIC coredumps, and it
contains a couple of fields under ifdef on CONFIG_BINFMT_ELF_FDPIC.
ELF is MMU-dependent and most, but not all configs that allow ELF_FDPIC
are non-MMU.  ARM is an exception - there ELF_FDPIC is allowed both for
MMU and non-MMU configs.  That's a problem - struct elf_prstatus is a
part of coredump layout, so ELF coredumps produced by arm kernels that
have ELF_FDPIC enabled are incompatible with those that have it disabled.

	The obvious solution is to introduce struct elf_prstatus_fdpic
and use that in binfmt_elf_fdpic.c, taking these fields out of the
normal struct elf_prstatus.  Unfortunately, the damn thing is defined in
include/uapi/linux/elfcore.h, so nominally it's a part of userland ABI.
However, not a single userland program actually includes linux/elfcore.h.
The reason is that the definition in there uses elf_gregset_t as a member,
and _that_ is not defined anywhere in the exported headers.  It is defined
in (libc) sys/procfs.h, but the same file defines struct elf_prstatus
as well.  So if you try to include linux/elfcore.h without having already
pulled sys/procfs.h, it'll break on incomplete type of a member.  And if
you have pulled sys/procfs.h, it'll break on redefining a structure.
IOW, it's not usable and it never had been; as the matter of fact,
that's the reason sys/procfs.h had been introduced back in 1996.

1/7) unexport linux/elfcore.h
	Takes it out of include/uapi/linux and moves the stuff that used
to live there into include/linux/elfcore.h

2/7) take fdpic-related parts of elf_prstatus out
	Now we can take that ifdef out of the definition of elf_prstatus
(as well as compat_elf_prstatus) and put the variant with those extra
fields into binfmt_elf_fdpic.c, calling it elf_prstatus_fdpic there.

3/7) kill elf_fpxregs_t
	All code dealing with it (both in elf_fdpic and non-regset side
of elf) is conditional upon ELF_CORE_COPY_XFPREGS.  And no architectures
define that anymore.  Take the dead code out.

4/7) [elf-fdpic] coredump: don't bother with cyclic list for per-thread
objects
5/7) [elf-fdpic] move allocation of elf_thread_status into
elf_dump_thread_status()
6/7) [elf-fdpic] use elf_dump_thread_status() for the dumper thread as well
	Massaging fdpic coredump logics towards the regset side of
elf coredump.

7/7) [elf-fdpic] switch coredump to regsets
	... and now we can switch from elf_core_copy_task_{,fp}regs()
to regset_get().


Diffstat:
 arch/ia64/include/asm/elf.h    |   2 -
 arch/powerpc/include/asm/elf.h |   2 -
 arch/x86/include/asm/elf.h     |   2 -
 fs/binfmt_elf.c                |  30 ------
 fs/binfmt_elf_fdpic.c          | 205 ++++++++++++++++++-----------------------
 include/linux/elfcore-compat.h |   4 -
 include/linux/elfcore.h        |  66 +++++++++++--
 include/uapi/linux/elfcore.h   | 101 --------------------
 scripts/headers_install.sh     |   1 -
 usr/include/Makefile           |   1 -
 10 files changed, 146 insertions(+), 268 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/7] unexport linux/elfcore.h
  2020-06-30  4:36 [RFC][PATCHES] converting FDPIC coredumps to regsets Al Viro
@ 2020-06-30  4:41 ` Al Viro
  2020-06-30  4:41   ` [PATCH 2/7] take fdpic-related parts of elf_prstatus out Al Viro
                     ` (5 more replies)
  2020-06-30 15:45 ` [RFC][PATCHES] converting FDPIC coredumps " Nicolas Pitre
  2020-07-14 17:14 ` Eric W. Biederman
  2 siblings, 6 replies; 10+ messages in thread
From: Al Viro @ 2020-06-30  4:41 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Eric W . Biederman, David Howells, Nicolas Pitre

From: Al Viro <viro@zeniv.linux.org.uk>

It's unusable from userland - it uses elf_gregset_t, which is not
provided by exported headers.  glibc has it in sys/procfs.h, but
the same file defines struct elf_prstatus, so linux/elfcore.h can't
be included once sys/procfs.h has been pulled.  Same goes for uclibc
and dietlibc simply doesn't have elf_gregset_t defined anywhere.

IOW, no userland source is including that thing.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/linux/elfcore.h      |  69 +++++++++++++++++++++++++++--
 include/uapi/linux/elfcore.h | 101 -------------------------------------------
 scripts/headers_install.sh   |   1 -
 usr/include/Makefile         |   1 -
 4 files changed, 66 insertions(+), 106 deletions(-)
 delete mode 100644 include/uapi/linux/elfcore.h

diff --git a/include/linux/elfcore.h b/include/linux/elfcore.h
index 4cad0e784b28..96ab215dad2d 100644
--- a/include/linux/elfcore.h
+++ b/include/linux/elfcore.h
@@ -5,12 +5,75 @@
 #include <linux/user.h>
 #include <linux/bug.h>
 #include <linux/sched/task_stack.h>
-
-#include <asm/elf.h>
-#include <uapi/linux/elfcore.h>
+#include <linux/types.h>
+#include <linux/signal.h>
+#include <linux/time.h>
+#include <linux/ptrace.h>
+#include <linux/fs.h>
+#include <linux/elf.h>
 
 struct coredump_params;
 
+struct elf_siginfo
+{
+	int	si_signo;			/* signal number */
+	int	si_code;			/* extra code */
+	int	si_errno;			/* errno */
+};
+
+/*
+ * Definitions to generate Intel SVR4-like core files.
+ * These mostly have the same names as the SVR4 types with "elf_"
+ * tacked on the front to prevent clashes with linux definitions,
+ * and the typedef forms have been avoided.  This is mostly like
+ * 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_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 */
+	elf_gregset_t pr_reg;	/* GP registers */
+#ifdef CONFIG_BINFMT_ELF_FDPIC
+	/* When using FDPIC, the loadmap addresses need to be communicated
+	 * to GDB in order for GDB to do the necessary relocations.  The
+	 * fields (below) used to communicate this information are placed
+	 * immediately after ``pr_reg'', so that the loadmap addresses may
+	 * be viewed as part of the register set if so desired.
+	 */
+	unsigned long pr_exec_fdpic_loadmap;
+	unsigned long pr_interp_fdpic_loadmap;
+#endif
+	int pr_fpvalid;		/* True if math co-processor being used.  */
+};
+
+#define ELF_PRARGSZ	(80)	/* Number of chars for args */
+
+struct elf_prpsinfo
+{
+	char	pr_state;	/* numeric process state */
+	char	pr_sname;	/* char for pr_state */
+	char	pr_zomb;	/* zombie */
+	char	pr_nice;	/* nice val */
+	unsigned long 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 */
+};
+
 static inline void elf_core_copy_regs(elf_gregset_t *elfregs, struct pt_regs *regs)
 {
 #ifdef ELF_CORE_COPY_REGS
diff --git a/include/uapi/linux/elfcore.h b/include/uapi/linux/elfcore.h
deleted file mode 100644
index baf03562306d..000000000000
--- a/include/uapi/linux/elfcore.h
+++ /dev/null
@@ -1,101 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _UAPI_LINUX_ELFCORE_H
-#define _UAPI_LINUX_ELFCORE_H
-
-#include <linux/types.h>
-#include <linux/signal.h>
-#include <linux/time.h>
-#include <linux/ptrace.h>
-#include <linux/elf.h>
-#include <linux/fs.h>
-
-struct elf_siginfo
-{
-	int	si_signo;			/* signal number */
-	int	si_code;			/* extra code */
-	int	si_errno;			/* errno */
-};
-
-
-#ifndef __KERNEL__
-typedef elf_greg_t greg_t;
-typedef elf_gregset_t gregset_t;
-typedef elf_fpregset_t fpregset_t;
-typedef elf_fpxregset_t fpxregset_t;
-#define NGREG ELF_NGREG
-#endif
-
-/*
- * Definitions to generate Intel SVR4-like core files.
- * These mostly have the same names as the SVR4 types with "elf_"
- * tacked on the front to prevent clashes with linux definitions,
- * and the typedef forms have been avoided.  This is mostly like
- * the SVR4 structure, but more Linuxy, with things that Linux does
- * not support and which gdb doesn't really use excluded.
- * Fields present but not used are marked with "XXX".
- */
-struct elf_prstatus
-{
-#if 0
-	long	pr_flags;	/* XXX Process flags */
-	short	pr_why;		/* XXX Reason for process halt */
-	short	pr_what;	/* XXX More detailed reason */
-#endif
-	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 */
-#if 0
-	struct sigaltstack pr_altstack;	/* Alternate stack info */
-	struct sigaction pr_action;	/* Signal action for current sig */
-#endif
-	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 */
-#if 0
-	long	pr_instr;		/* Current instruction */
-#endif
-	elf_gregset_t pr_reg;	/* GP registers */
-#ifdef CONFIG_BINFMT_ELF_FDPIC
-	/* When using FDPIC, the loadmap addresses need to be communicated
-	 * to GDB in order for GDB to do the necessary relocations.  The
-	 * fields (below) used to communicate this information are placed
-	 * immediately after ``pr_reg'', so that the loadmap addresses may
-	 * be viewed as part of the register set if so desired.
-	 */
-	unsigned long pr_exec_fdpic_loadmap;
-	unsigned long pr_interp_fdpic_loadmap;
-#endif
-	int pr_fpvalid;		/* True if math co-processor being used.  */
-};
-
-#define ELF_PRARGSZ	(80)	/* Number of chars for args */
-
-struct elf_prpsinfo
-{
-	char	pr_state;	/* numeric process state */
-	char	pr_sname;	/* char for pr_state */
-	char	pr_zomb;	/* zombie */
-	char	pr_nice;	/* nice val */
-	unsigned long 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 */
-};
-
-#ifndef __KERNEL__
-typedef struct elf_prstatus prstatus_t;
-typedef struct elf_prpsinfo prpsinfo_t;
-#define PRARGSZ ELF_PRARGSZ 
-#endif
-
-
-#endif /* _UAPI_LINUX_ELFCORE_H */
diff --git a/scripts/headers_install.sh b/scripts/headers_install.sh
index 955cf3aedf21..9314247bb222 100755
--- a/scripts/headers_install.sh
+++ b/scripts/headers_install.sh
@@ -86,7 +86,6 @@ arch/x86/include/uapi/asm/auxvec.h:CONFIG_X86_64
 arch/x86/include/uapi/asm/mman.h:CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
 include/uapi/asm-generic/fcntl.h:CONFIG_64BIT
 include/uapi/linux/atmdev.h:CONFIG_COMPAT
-include/uapi/linux/elfcore.h:CONFIG_BINFMT_ELF_FDPIC
 include/uapi/linux/eventpoll.h:CONFIG_PM_SLEEP
 include/uapi/linux/hw_breakpoint.h:CONFIG_HAVE_MIXED_BREAKPOINTS_REGS
 include/uapi/linux/pktcdvd.h:CONFIG_CDROM_PKTCDVD_WCACHE
diff --git a/usr/include/Makefile b/usr/include/Makefile
index 55362f3ab393..f6b3c85d900e 100644
--- a/usr/include/Makefile
+++ b/usr/include/Makefile
@@ -28,7 +28,6 @@ no-header-test += linux/am437x-vpfe.h
 no-header-test += linux/android/binder.h
 no-header-test += linux/android/binderfs.h
 no-header-test += linux/coda.h
-no-header-test += linux/elfcore.h
 no-header-test += linux/errqueue.h
 no-header-test += linux/fsmap.h
 no-header-test += linux/hdlc/ioctl.h
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/7] take fdpic-related parts of elf_prstatus out
  2020-06-30  4:41 ` [PATCH 1/7] unexport linux/elfcore.h Al Viro
@ 2020-06-30  4:41   ` Al Viro
  2020-06-30  4:41   ` [PATCH 3/7] kill elf_fpxregs_t Al Viro
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Al Viro @ 2020-06-30  4:41 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Eric W . Biederman, David Howells, Nicolas Pitre

From: Al Viro <viro@zeniv.linux.org.uk>

The only architecture where we might end up using both is arm,
and there we definitely don't want fdpic-related fields in
elf_prstatus - coredump layout of ELF binaries should not
depend upon having the kernel built with the support of ELF_FDPIC
ones.  Just move the fdpic-modified variant into binfmt_elf_fdpic.c
(and call it elf_prstatus_fdpic there)

[name stolen from nico]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/binfmt_elf_fdpic.c          | 32 +++++++++++++++++++++++++++++---
 include/linux/elfcore-compat.h |  4 ----
 include/linux/elfcore.h        | 10 ----------
 3 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 0f45521b237c..6e13d8bea32d 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1189,6 +1189,32 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *params,
  */
 #ifdef CONFIG_ELF_CORE
 
+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 */
+	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
+	 * fields (below) used to communicate this information are placed
+	 * immediately after ``pr_reg'', so that the loadmap addresses may
+	 * be viewed as part of the register set if so desired.
+	 */
+	unsigned long pr_exec_fdpic_loadmap;
+	unsigned long pr_interp_fdpic_loadmap;
+	int pr_fpvalid;		/* True if math co-processor being used.  */
+};
+
 /*
  * Decide whether a segment is worth dumping; default is yes to be
  * sure (missing info is worse than too much; etc).
@@ -1345,7 +1371,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 *prstatus,
+static void fill_prstatus(struct elf_prstatus_fdpic *prstatus,
 			  struct task_struct *p, long signr)
 {
 	prstatus->pr_info.si_signo = prstatus->pr_cursig = signr;
@@ -1428,7 +1454,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
 struct elf_thread_status
 {
 	struct list_head list;
-	struct elf_prstatus prstatus;	/* NT_PRSTATUS */
+	struct elf_prstatus_fdpic prstatus;	/* NT_PRSTATUS */
 	elf_fpregset_t fpu;		/* NT_PRFPREG */
 	struct task_struct *thread;
 #ifdef ELF_CORE_COPY_XFPREGS
@@ -1562,7 +1588,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	loff_t offset = 0, dataoff;
 	int numnote;
 	struct memelfnote *notes = NULL;
-	struct elf_prstatus *prstatus = NULL;	/* NT_PRSTATUS */
+	struct elf_prstatus_fdpic *prstatus = NULL;	/* NT_PRSTATUS */
 	struct elf_prpsinfo *psinfo = NULL;	/* NT_PRPSINFO */
  	LIST_HEAD(thread_list);
  	struct list_head *t;
diff --git a/include/linux/elfcore-compat.h b/include/linux/elfcore-compat.h
index 7a37f4ce9fd2..10485f0c9740 100644
--- a/include/linux/elfcore-compat.h
+++ b/include/linux/elfcore-compat.h
@@ -32,10 +32,6 @@ struct compat_elf_prstatus
 	struct old_timeval32		pr_cutime;
 	struct old_timeval32		pr_cstime;
 	compat_elf_gregset_t		pr_reg;
-#ifdef CONFIG_BINFMT_ELF_FDPIC
-	compat_ulong_t			pr_exec_fdpic_loadmap;
-	compat_ulong_t			pr_interp_fdpic_loadmap;
-#endif
 	compat_int_t			pr_fpvalid;
 };
 
diff --git a/include/linux/elfcore.h b/include/linux/elfcore.h
index 96ab215dad2d..adb8ee89f3fd 100644
--- a/include/linux/elfcore.h
+++ b/include/linux/elfcore.h
@@ -44,16 +44,6 @@ struct elf_prstatus
 	struct __kernel_old_timeval pr_cutime;	/* Cumulative user time */
 	struct __kernel_old_timeval pr_cstime;	/* Cumulative system time */
 	elf_gregset_t pr_reg;	/* GP registers */
-#ifdef CONFIG_BINFMT_ELF_FDPIC
-	/* When using FDPIC, the loadmap addresses need to be communicated
-	 * to GDB in order for GDB to do the necessary relocations.  The
-	 * fields (below) used to communicate this information are placed
-	 * immediately after ``pr_reg'', so that the loadmap addresses may
-	 * be viewed as part of the register set if so desired.
-	 */
-	unsigned long pr_exec_fdpic_loadmap;
-	unsigned long pr_interp_fdpic_loadmap;
-#endif
 	int pr_fpvalid;		/* True if math co-processor being used.  */
 };
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/7] kill elf_fpxregs_t
  2020-06-30  4:41 ` [PATCH 1/7] unexport linux/elfcore.h Al Viro
  2020-06-30  4:41   ` [PATCH 2/7] take fdpic-related parts of elf_prstatus out Al Viro
@ 2020-06-30  4:41   ` Al Viro
  2020-06-30  4:41   ` [PATCH 4/7] [elf-fdpic] coredump: don't bother with cyclic list for per-thread objects Al Viro
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Al Viro @ 2020-06-30  4:41 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Eric W . Biederman, David Howells, Nicolas Pitre

From: Al Viro <viro@zeniv.linux.org.uk>

all uses are conditional upon ELF_CORE_COPY_XFPREGS, which has not
been defined on any architecture since 2010

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/ia64/include/asm/elf.h    |  2 --
 arch/powerpc/include/asm/elf.h |  2 --
 arch/x86/include/asm/elf.h     |  2 --
 fs/binfmt_elf.c                | 30 ------------------------------
 fs/binfmt_elf_fdpic.c          | 28 ----------------------------
 include/linux/elfcore.h        |  7 -------
 6 files changed, 71 deletions(-)

diff --git a/arch/ia64/include/asm/elf.h b/arch/ia64/include/asm/elf.h
index c70bb9c11f52..6629301a2620 100644
--- a/arch/ia64/include/asm/elf.h
+++ b/arch/ia64/include/asm/elf.h
@@ -179,8 +179,6 @@ extern void ia64_init_addr_space (void);
 #define ELF_AR_SSD_OFFSET  (56 * sizeof(elf_greg_t))
 #define ELF_AR_END_OFFSET  (57 * sizeof(elf_greg_t))
 
-typedef unsigned long elf_fpxregset_t;
-
 typedef unsigned long elf_greg_t;
 typedef elf_greg_t elf_gregset_t[ELF_NGREG];
 
diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
index 57c229a86f08..53ed2ca40151 100644
--- a/arch/powerpc/include/asm/elf.h
+++ b/arch/powerpc/include/asm/elf.h
@@ -53,8 +53,6 @@ static inline void ppc_elf_core_copy_regs(elf_gregset_t elf_regs,
 }
 #define ELF_CORE_COPY_REGS(gregs, regs) ppc_elf_core_copy_regs(gregs, regs);
 
-typedef elf_vrregset_t elf_fpxregset_t;
-
 /* ELF_HWCAP yields a mask that user programs can use to figure out what
    instruction set this cpu supports.  This could be done in userspace,
    but it's not easy, and we've already done it here.  */
diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 452beed7892b..b9a5d488f1a5 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -21,8 +21,6 @@ typedef struct user_i387_struct elf_fpregset_t;
 
 #ifdef __i386__
 
-typedef struct user_fxsr_struct elf_fpxregset_t;
-
 #define R_386_NONE	0
 #define R_386_32	1
 #define R_386_PC32	2
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 80743b8957c9..6a171a28bdf7 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -2038,9 +2038,6 @@ struct elf_thread_status
 	struct elf_prstatus prstatus;	/* NT_PRSTATUS */
 	elf_fpregset_t fpu;		/* NT_PRFPREG */
 	struct task_struct *thread;
-#ifdef ELF_CORE_COPY_XFPREGS
-	elf_fpxregset_t xfpu;		/* ELF_CORE_XFPREG_TYPE */
-#endif
 	struct memelfnote notes[3];
 	int num_notes;
 };
@@ -2071,15 +2068,6 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
 		t->num_notes++;
 		sz += notesize(&t->notes[1]);
 	}
-
-#ifdef ELF_CORE_COPY_XFPREGS
-	if (elf_core_copy_task_xfpregs(p, &t->xfpu)) {
-		fill_note(&t->notes[2], "LINUX", ELF_CORE_XFPREG_TYPE,
-			  sizeof(t->xfpu), &t->xfpu);
-		t->num_notes++;
-		sz += notesize(&t->notes[2]);
-	}
-#endif	
 	return sz;
 }
 
@@ -2090,9 +2078,6 @@ struct elf_note_info {
 	struct elf_prpsinfo *psinfo;	/* NT_PRPSINFO */
 	struct list_head thread_list;
 	elf_fpregset_t *fpu;
-#ifdef ELF_CORE_COPY_XFPREGS
-	elf_fpxregset_t *xfpu;
-#endif
 	user_siginfo_t csigdata;
 	int thread_status_size;
 	int numnote;
@@ -2116,11 +2101,6 @@ static int elf_note_info_init(struct elf_note_info *info)
 	info->fpu = kmalloc(sizeof(*info->fpu), GFP_KERNEL);
 	if (!info->fpu)
 		return 0;
-#ifdef ELF_CORE_COPY_XFPREGS
-	info->xfpu = kmalloc(sizeof(*info->xfpu), GFP_KERNEL);
-	if (!info->xfpu)
-		return 0;
-#endif
 	return 1;
 }
 
@@ -2184,13 +2164,6 @@ static int fill_note_info(struct elfhdr *elf, int phdrs,
 	if (info->prstatus->pr_fpvalid)
 		fill_note(info->notes + info->numnote++,
 			  "CORE", NT_PRFPREG, sizeof(*info->fpu), info->fpu);
-#ifdef ELF_CORE_COPY_XFPREGS
-	if (elf_core_copy_task_xfpregs(current, info->xfpu))
-		fill_note(info->notes + info->numnote++,
-			  "LINUX", ELF_CORE_XFPREG_TYPE,
-			  sizeof(*info->xfpu), info->xfpu);
-#endif
-
 	return 1;
 }
 
@@ -2243,9 +2216,6 @@ static void free_note_info(struct elf_note_info *info)
 	kfree(info->psinfo);
 	kfree(info->notes);
 	kfree(info->fpu);
-#ifdef ELF_CORE_COPY_XFPREGS
-	kfree(info->xfpu);
-#endif
 }
 
 #endif
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 6e13d8bea32d..a6ee92137529 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1457,9 +1457,6 @@ struct elf_thread_status
 	struct elf_prstatus_fdpic prstatus;	/* NT_PRSTATUS */
 	elf_fpregset_t fpu;		/* NT_PRFPREG */
 	struct task_struct *thread;
-#ifdef ELF_CORE_COPY_XFPREGS
-	elf_fpxregset_t xfpu;		/* ELF_CORE_XFPREG_TYPE */
-#endif
 	struct memelfnote notes[3];
 	int num_notes;
 };
@@ -1491,15 +1488,6 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
 		t->num_notes++;
 		sz += notesize(&t->notes[1]);
 	}
-
-#ifdef ELF_CORE_COPY_XFPREGS
-	if (elf_core_copy_task_xfpregs(p, &t->xfpu)) {
-		fill_note(&t->notes[2], "LINUX", ELF_CORE_XFPREG_TYPE,
-			  sizeof(t->xfpu), &t->xfpu);
-		t->num_notes++;
-		sz += notesize(&t->notes[2]);
-	}
-#endif
 	return sz;
 }
 
@@ -1593,9 +1581,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
  	LIST_HEAD(thread_list);
  	struct list_head *t;
 	elf_fpregset_t *fpu = NULL;
-#ifdef ELF_CORE_COPY_XFPREGS
-	elf_fpxregset_t *xfpu = NULL;
-#endif
 	int thread_status_size = 0;
 	elf_addr_t *auxv;
 	struct elf_phdr *phdr4note = NULL;
@@ -1634,11 +1619,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	fpu = kmalloc(sizeof(*fpu), GFP_KERNEL);
 	if (!fpu)
 		goto end_coredump;
-#ifdef ELF_CORE_COPY_XFPREGS
-	xfpu = kmalloc(sizeof(*xfpu), GFP_KERNEL);
-	if (!xfpu)
-		goto end_coredump;
-#endif
 
 	for (ct = current->mm->core_state->dumper.next;
 					ct; ct = ct->next) {
@@ -1703,11 +1683,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	     elf_core_copy_task_fpregs(current, cprm->regs, fpu)))
 		fill_note(notes + numnote++,
 			  "CORE", NT_PRFPREG, sizeof(*fpu), fpu);
-#ifdef ELF_CORE_COPY_XFPREGS
-	if (elf_core_copy_task_xfpregs(current, xfpu))
-		fill_note(notes + numnote++,
-			  "LINUX", ELF_CORE_XFPREG_TYPE, sizeof(*xfpu), xfpu);
-#endif
 
 	offset += sizeof(*elf);				/* Elf header */
 	offset += segs * sizeof(struct elf_phdr);	/* Program headers */
@@ -1828,9 +1803,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	kfree(notes);
 	kfree(fpu);
 	kfree(shdr4extnum);
-#ifdef ELF_CORE_COPY_XFPREGS
-	kfree(xfpu);
-#endif
 	return has_dumped;
 #undef NUM_NOTES
 }
diff --git a/include/linux/elfcore.h b/include/linux/elfcore.h
index adb8ee89f3fd..46c3d691f677 100644
--- a/include/linux/elfcore.h
+++ b/include/linux/elfcore.h
@@ -104,13 +104,6 @@ static inline int elf_core_copy_task_fpregs(struct task_struct *t, struct pt_reg
 #endif
 }
 
-#ifdef ELF_CORE_COPY_XFPREGS
-static inline int elf_core_copy_task_xfpregs(struct task_struct *t, elf_fpxregset_t *xfpu)
-{
-	return ELF_CORE_COPY_XFPREGS(t, xfpu);
-}
-#endif
-
 /*
  * These functions parameterize elf_core_dump in fs/binfmt_elf.c to write out
  * extra segments containing the gate DSO contents.  Dumping its
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/7] [elf-fdpic] coredump: don't bother with cyclic list for per-thread objects
  2020-06-30  4:41 ` [PATCH 1/7] unexport linux/elfcore.h Al Viro
  2020-06-30  4:41   ` [PATCH 2/7] take fdpic-related parts of elf_prstatus out Al Viro
  2020-06-30  4:41   ` [PATCH 3/7] kill elf_fpxregs_t Al Viro
@ 2020-06-30  4:41   ` Al Viro
  2020-06-30  4:41   ` [PATCH 5/7] [elf-fdpic] move allocation of elf_thread_status into elf_dump_thread_status() Al Viro
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Al Viro @ 2020-06-30  4:41 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Eric W . Biederman, David Howells, Nicolas Pitre

From: Al Viro <viro@zeniv.linux.org.uk>

plain single-linked list is just fine here...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/binfmt_elf_fdpic.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index a6ee92137529..bcbf756fba39 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1453,7 +1453,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
 /* Here is the structure in which status of each thread is captured. */
 struct elf_thread_status
 {
-	struct list_head list;
+	struct elf_thread_status *next;
 	struct elf_prstatus_fdpic prstatus;	/* NT_PRSTATUS */
 	elf_fpregset_t fpu;		/* NT_PRFPREG */
 	struct task_struct *thread;
@@ -1578,8 +1578,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	struct memelfnote *notes = NULL;
 	struct elf_prstatus_fdpic *prstatus = NULL;	/* NT_PRSTATUS */
 	struct elf_prpsinfo *psinfo = NULL;	/* NT_PRPSINFO */
- 	LIST_HEAD(thread_list);
- 	struct list_head *t;
+	struct elf_thread_status *thread_list = NULL;
 	elf_fpregset_t *fpu = NULL;
 	int thread_status_size = 0;
 	elf_addr_t *auxv;
@@ -1627,15 +1626,12 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 			goto end_coredump;
 
 		tmp->thread = ct->task;
-		list_add(&tmp->list, &thread_list);
+		tmp->next = thread_list;
+		thread_list = tmp;
 	}
 
-	list_for_each(t, &thread_list) {
-		struct elf_thread_status *tmp;
-		int sz;
-
-		tmp = list_entry(t, struct elf_thread_status, list);
-		sz = elf_dump_thread_status(cprm->siginfo->si_signo, tmp);
+	for (tmp = thread_list; tmp; tmp = tmp->next) {
+		int sz = elf_dump_thread_status(cprm->siginfo->si_signo, tmp);
 		thread_status_size += sz;
 	}
 
@@ -1760,10 +1756,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 			goto end_coredump;
 
 	/* write out the thread status notes section */
-	list_for_each(t, &thread_list) {
-		struct elf_thread_status *tmp =
-				list_entry(t, struct elf_thread_status, list);
-
+	for (tmp = thread_list; tmp; tmp = tmp->next) {
 		for (i = 0; i < tmp->num_notes; i++)
 			if (!writenote(&tmp->notes[i], cprm))
 				goto end_coredump;
@@ -1791,10 +1784,10 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	}
 
 end_coredump:
-	while (!list_empty(&thread_list)) {
-		struct list_head *tmp = thread_list.next;
-		list_del(tmp);
-		kfree(list_entry(tmp, struct elf_thread_status, list));
+	while (thread_list) {
+		tmp = thread_list;
+		thread_list = thread_list->next;
+		kfree(tmp);
 	}
 	kfree(phdr4note);
 	kfree(elf);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/7] [elf-fdpic] move allocation of elf_thread_status into elf_dump_thread_status()
  2020-06-30  4:41 ` [PATCH 1/7] unexport linux/elfcore.h Al Viro
                     ` (2 preceding siblings ...)
  2020-06-30  4:41   ` [PATCH 4/7] [elf-fdpic] coredump: don't bother with cyclic list for per-thread objects Al Viro
@ 2020-06-30  4:41   ` Al Viro
  2020-06-30  4:41   ` [PATCH 6/7] [elf-fdpic] use elf_dump_thread_status() for the dumper thread as well Al Viro
  2020-06-30  4:41   ` [PATCH 7/7] [elf-fdpic] switch coredump to regsets Al Viro
  5 siblings, 0 replies; 10+ messages in thread
From: Al Viro @ 2020-06-30  4:41 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Eric W . Biederman, David Howells, Nicolas Pitre

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/binfmt_elf_fdpic.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index bcbf756fba39..ba4f264dff3a 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1466,12 +1466,13 @@ struct elf_thread_status
  * we need to keep a linked list of every thread's pr_status and then create
  * a single section for them in the final core file.
  */
-static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
+static struct elf_thread_status *elf_dump_thread_status(long signr, struct task_struct *p, int *sz)
 {
-	struct task_struct *p = t->thread;
-	int sz = 0;
+	struct elf_thread_status *t;
 
-	t->num_notes = 0;
+	t = kzalloc(sizeof(struct elf_thread_status), GFP_KERNEL);
+	if (!t)
+		return t;
 
 	fill_prstatus(&t->prstatus, p, signr);
 	elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
@@ -1479,16 +1480,16 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
 	fill_note(&t->notes[0], "CORE", NT_PRSTATUS, sizeof(t->prstatus),
 		  &t->prstatus);
 	t->num_notes++;
-	sz += notesize(&t->notes[0]);
+	*sz += notesize(&t->notes[0]);
 
 	t->prstatus.pr_fpvalid = elf_core_copy_task_fpregs(p, NULL, &t->fpu);
 	if (t->prstatus.pr_fpvalid) {
 		fill_note(&t->notes[1], "CORE", NT_PRFPREG, sizeof(t->fpu),
 			  &t->fpu);
 		t->num_notes++;
-		sz += notesize(&t->notes[1]);
+		*sz += notesize(&t->notes[1]);
 	}
-	return sz;
+	return t;
 }
 
 static void fill_extnum_info(struct elfhdr *elf, struct elf_shdr *shdr4extnum,
@@ -1621,20 +1622,15 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 
 	for (ct = current->mm->core_state->dumper.next;
 					ct; ct = ct->next) {
-		tmp = kzalloc(sizeof(*tmp), GFP_KERNEL);
+		tmp = elf_dump_thread_status(cprm->siginfo->si_signo,
+					     ct->task, &thread_status_size);
 		if (!tmp)
 			goto end_coredump;
 
-		tmp->thread = ct->task;
 		tmp->next = thread_list;
 		thread_list = tmp;
 	}
 
-	for (tmp = thread_list; tmp; tmp = tmp->next) {
-		int sz = elf_dump_thread_status(cprm->siginfo->si_signo, tmp);
-		thread_status_size += sz;
-	}
-
 	/* now collect the dump for the current */
 	fill_prstatus(prstatus, current, cprm->siginfo->si_signo);
 	elf_core_copy_regs(&prstatus->pr_reg, cprm->regs);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/7] [elf-fdpic] use elf_dump_thread_status() for the dumper thread as well
  2020-06-30  4:41 ` [PATCH 1/7] unexport linux/elfcore.h Al Viro
                     ` (3 preceding siblings ...)
  2020-06-30  4:41   ` [PATCH 5/7] [elf-fdpic] move allocation of elf_thread_status into elf_dump_thread_status() Al Viro
@ 2020-06-30  4:41   ` Al Viro
  2020-06-30  4:41   ` [PATCH 7/7] [elf-fdpic] switch coredump to regsets Al Viro
  5 siblings, 0 replies; 10+ messages in thread
From: Al Viro @ 2020-06-30  4:41 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Eric W . Biederman, David Howells, Nicolas Pitre

From: Al Viro <viro@zeniv.linux.org.uk>

the only reason to have it open-coded for the first (dumper) thread is
that coredump has a couple of process-wide notes stuck right after the
first (NT_PRSTATUS) note of the first thread.  But we don't need to
make the data collection side irregular for the first thread to handle
that - it's only the logics ordering the calls of writenote() that
needs to take care of that.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/binfmt_elf_fdpic.c | 81 ++++++++++++++++++---------------------------------
 1 file changed, 28 insertions(+), 53 deletions(-)

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index ba4f264dff3a..34c45410d587 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1482,7 +1482,8 @@ static struct elf_thread_status *elf_dump_thread_status(long signr, struct task_
 	t->num_notes++;
 	*sz += notesize(&t->notes[0]);
 
-	t->prstatus.pr_fpvalid = elf_core_copy_task_fpregs(p, NULL, &t->fpu);
+	t->prstatus.pr_fpvalid = elf_core_copy_task_fpregs(p, task_pt_regs(p),
+							   &t->fpu);
 	if (t->prstatus.pr_fpvalid) {
 		fill_note(&t->notes[1], "CORE", NT_PRFPREG, sizeof(t->fpu),
 			  &t->fpu);
@@ -1568,19 +1569,15 @@ static size_t elf_core_vma_data_size(unsigned long mm_flags)
  */
 static int elf_fdpic_core_dump(struct coredump_params *cprm)
 {
-#define	NUM_NOTES	6
 	int has_dumped = 0;
 	int segs;
 	int i;
 	struct vm_area_struct *vma;
 	struct elfhdr *elf = NULL;
 	loff_t offset = 0, dataoff;
-	int numnote;
-	struct memelfnote *notes = NULL;
-	struct elf_prstatus_fdpic *prstatus = NULL;	/* NT_PRSTATUS */
+	struct memelfnote psinfo_note, auxv_note;
 	struct elf_prpsinfo *psinfo = NULL;	/* NT_PRPSINFO */
 	struct elf_thread_status *thread_list = NULL;
-	elf_fpregset_t *fpu = NULL;
 	int thread_status_size = 0;
 	elf_addr_t *auxv;
 	struct elf_phdr *phdr4note = NULL;
@@ -1606,19 +1603,9 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	elf = kmalloc(sizeof(*elf), GFP_KERNEL);
 	if (!elf)
 		goto end_coredump;
-	prstatus = kzalloc(sizeof(*prstatus), GFP_KERNEL);
-	if (!prstatus)
-		goto end_coredump;
 	psinfo = kmalloc(sizeof(*psinfo), GFP_KERNEL);
 	if (!psinfo)
 		goto end_coredump;
-	notes = kmalloc_array(NUM_NOTES, sizeof(struct memelfnote),
-			      GFP_KERNEL);
-	if (!notes)
-		goto end_coredump;
-	fpu = kmalloc(sizeof(*fpu), GFP_KERNEL);
-	if (!fpu)
-		goto end_coredump;
 
 	for (ct = current->mm->core_state->dumper.next;
 					ct; ct = ct->next) {
@@ -1632,8 +1619,12 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	}
 
 	/* now collect the dump for the current */
-	fill_prstatus(prstatus, current, cprm->siginfo->si_signo);
-	elf_core_copy_regs(&prstatus->pr_reg, cprm->regs);
+	tmp = elf_dump_thread_status(cprm->siginfo->si_signo,
+				     current, &thread_status_size);
+	if (!tmp)
+		goto end_coredump;
+	tmp->next = thread_list;
+	thread_list = tmp;
 
 	segs = current->mm->map_count;
 	segs += elf_core_extra_phdrs();
@@ -1655,46 +1646,28 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	 * with info from their /proc.
 	 */
 
-	fill_note(notes + 0, "CORE", NT_PRSTATUS, sizeof(*prstatus), prstatus);
 	fill_psinfo(psinfo, current->group_leader, current->mm);
-	fill_note(notes + 1, "CORE", NT_PRPSINFO, sizeof(*psinfo), psinfo);
-
-	numnote = 2;
+	fill_note(&psinfo_note, "CORE", NT_PRPSINFO, sizeof(*psinfo), psinfo);
+	thread_status_size += notesize(&psinfo_note);
 
 	auxv = (elf_addr_t *) current->mm->saved_auxv;
-
 	i = 0;
 	do
 		i += 2;
 	while (auxv[i - 2] != AT_NULL);
-	fill_note(&notes[numnote++], "CORE", NT_AUXV,
-		  i * sizeof(elf_addr_t), auxv);
+	fill_note(&auxv_note, "CORE", NT_AUXV, i * sizeof(elf_addr_t), auxv);
+	thread_status_size += notesize(&auxv_note);
 
-  	/* Try to dump the FPU. */
-	if ((prstatus->pr_fpvalid =
-	     elf_core_copy_task_fpregs(current, cprm->regs, fpu)))
-		fill_note(notes + numnote++,
-			  "CORE", NT_PRFPREG, sizeof(*fpu), fpu);
-
-	offset += sizeof(*elf);				/* Elf header */
+	offset = sizeof(*elf);				/* Elf header */
 	offset += segs * sizeof(struct elf_phdr);	/* Program headers */
 
 	/* Write notes phdr entry */
-	{
-		int sz = 0;
-
-		for (i = 0; i < numnote; i++)
-			sz += notesize(notes + i);
-
-		sz += thread_status_size;
-
-		phdr4note = kmalloc(sizeof(*phdr4note), GFP_KERNEL);
-		if (!phdr4note)
-			goto end_coredump;
+	phdr4note = kmalloc(sizeof(*phdr4note), GFP_KERNEL);
+	if (!phdr4note)
+		goto end_coredump;
 
-		fill_elf_note_phdr(phdr4note, sz, offset);
-		offset += sz;
-	}
+	fill_elf_note_phdr(phdr4note, thread_status_size, offset);
+	offset += thread_status_size;
 
 	/* Page-align dumped data */
 	dataoff = offset = roundup(offset, ELF_EXEC_PAGESIZE);
@@ -1747,12 +1720,18 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 		goto end_coredump;
 
  	/* write out the notes section */
-	for (i = 0; i < numnote; i++)
-		if (!writenote(notes + i, cprm))
+	if (!writenote(thread_list->notes, cprm))
+		goto end_coredump;
+	if (!writenote(&psinfo_note, cprm))
+		goto end_coredump;
+	if (!writenote(&auxv_note, cprm))
+		goto end_coredump;
+	for (i = 1; i < thread_list->num_notes; i++)
+		if (!writenote(thread_list->notes + i, cprm))
 			goto end_coredump;
 
 	/* write out the thread status notes section */
-	for (tmp = thread_list; tmp; tmp = tmp->next) {
+	for (tmp = thread_list->next; tmp; tmp = tmp->next) {
 		for (i = 0; i < tmp->num_notes; i++)
 			if (!writenote(&tmp->notes[i], cprm))
 				goto end_coredump;
@@ -1787,13 +1766,9 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	}
 	kfree(phdr4note);
 	kfree(elf);
-	kfree(prstatus);
 	kfree(psinfo);
-	kfree(notes);
-	kfree(fpu);
 	kfree(shdr4extnum);
 	return has_dumped;
-#undef NUM_NOTES
 }
 
 #endif		/* CONFIG_ELF_CORE */
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/7] [elf-fdpic] switch coredump to regsets
  2020-06-30  4:41 ` [PATCH 1/7] unexport linux/elfcore.h Al Viro
                     ` (4 preceding siblings ...)
  2020-06-30  4:41   ` [PATCH 6/7] [elf-fdpic] use elf_dump_thread_status() for the dumper thread as well Al Viro
@ 2020-06-30  4:41   ` Al Viro
  5 siblings, 0 replies; 10+ messages in thread
From: Al Viro @ 2020-06-30  4:41 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Eric W . Biederman, David Howells, Nicolas Pitre

From: Al Viro <viro@zeniv.linux.org.uk>

similar to how elf coredump is working on architectures that
have regsets, and all architectures with elf-fdpic support *do*
have that.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/binfmt_elf_fdpic.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 34c45410d587..1af03c8d3c09 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -35,6 +35,7 @@
 #include <linux/elfcore.h>
 #include <linux/coredump.h>
 #include <linux/dax.h>
+#include <linux/regset.h>
 
 #include <linux/uaccess.h>
 #include <asm/param.h>
@@ -1456,8 +1457,7 @@ struct elf_thread_status
 	struct elf_thread_status *next;
 	struct elf_prstatus_fdpic prstatus;	/* NT_PRSTATUS */
 	elf_fpregset_t fpu;		/* NT_PRFPREG */
-	struct task_struct *thread;
-	struct memelfnote notes[3];
+	struct memelfnote notes[2];
 	int num_notes;
 };
 
@@ -1468,22 +1468,35 @@ struct elf_thread_status
  */
 static struct elf_thread_status *elf_dump_thread_status(long signr, struct task_struct *p, int *sz)
 {
+	const struct user_regset_view *view = task_user_regset_view(p);
 	struct elf_thread_status *t;
+	int i, ret;
 
 	t = kzalloc(sizeof(struct elf_thread_status), GFP_KERNEL);
 	if (!t)
 		return t;
 
 	fill_prstatus(&t->prstatus, p, signr);
-	elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
+	regset_get(p, &view->regsets[0],
+		   sizeof(t->prstatus.pr_reg), &t->prstatus.pr_reg);
 
 	fill_note(&t->notes[0], "CORE", NT_PRSTATUS, sizeof(t->prstatus),
 		  &t->prstatus);
 	t->num_notes++;
 	*sz += notesize(&t->notes[0]);
 
-	t->prstatus.pr_fpvalid = elf_core_copy_task_fpregs(p, task_pt_regs(p),
-							   &t->fpu);
+	for (i = 1; i < view->n; ++i) {
+		const struct user_regset *regset = &view->regsets[i];
+		if (regset->core_note_type != NT_PRFPREG)
+			continue;
+		if (regset->active && regset->active(p, regset) <= 0)
+			continue;
+		ret = regset_get(p, regset, sizeof(t->fpu), &t->fpu);
+		if (ret >= 0)
+			t->prstatus.pr_fpvalid = 1;
+		break;
+	}
+
 	if (t->prstatus.pr_fpvalid) {
 		fill_note(&t->notes[1], "CORE", NT_PRFPREG, sizeof(t->fpu),
 			  &t->fpu);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCHES] converting FDPIC coredumps to regsets
  2020-06-30  4:36 [RFC][PATCHES] converting FDPIC coredumps to regsets Al Viro
  2020-06-30  4:41 ` [PATCH 1/7] unexport linux/elfcore.h Al Viro
@ 2020-06-30 15:45 ` Nicolas Pitre
  2020-07-14 17:14 ` Eric W. Biederman
  2 siblings, 0 replies; 10+ messages in thread
From: Nicolas Pitre @ 2020-06-30 15:45 UTC (permalink / raw)
  To: Al Viro; +Cc: Linus Torvalds, linux-kernel, Eric W. Biederman, David Howells

On Tue, 30 Jun 2020, Al Viro wrote:

> 	The obvious solution is to introduce struct elf_prstatus_fdpic
> and use that in binfmt_elf_fdpic.c, taking these fields out of the
> normal struct elf_prstatus.  Unfortunately, the damn thing is defined in
> include/uapi/linux/elfcore.h, so nominally it's a part of userland ABI.
> However, not a single userland program actually includes linux/elfcore.h.
> The reason is that the definition in there uses elf_gregset_t as a member,
> and _that_ is not defined anywhere in the exported headers.  It is defined
> in (libc) sys/procfs.h, but the same file defines struct elf_prstatus
> as well.  So if you try to include linux/elfcore.h without having already
> pulled sys/procfs.h, it'll break on incomplete type of a member.  And if
> you have pulled sys/procfs.h, it'll break on redefining a structure.
> IOW, it's not usable and it never had been; as the matter of fact,
> that's the reason sys/procfs.h had been introduced back in 1996.

Huh! That's convenient alright.

Acked-by: Nicolas Pitre <nico@fluxnic.net>


Nicolas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCHES] converting FDPIC coredumps to regsets
  2020-06-30  4:36 [RFC][PATCHES] converting FDPIC coredumps to regsets Al Viro
  2020-06-30  4:41 ` [PATCH 1/7] unexport linux/elfcore.h Al Viro
  2020-06-30 15:45 ` [RFC][PATCHES] converting FDPIC coredumps " Nicolas Pitre
@ 2020-07-14 17:14 ` Eric W. Biederman
  2 siblings, 0 replies; 10+ messages in thread
From: Eric W. Biederman @ 2020-07-14 17:14 UTC (permalink / raw)
  To: Al Viro; +Cc: Linus Torvalds, linux-kernel, David Howells, Nicolas Pitre

Al Viro <viro@zeniv.linux.org.uk> writes:

> 	Conversion of ELF coredumps to regsets has not touched
> ELF_FDPIC.  Right now all architectures that support FDPIC have
> regsets sufficient for switching it to regset-based coredumps.  A bit
> of backstory: original ELF (and ELF_FDPIC) coredumps reused the old
> helpers used by a.out coredumps.  These days a.out coredumps are gone;
> we could remove the dead code, if not for several obstacles.  And one
> of those obstacles is ELF_FDPIC.
>
> 	This series more or less reproduces the conversion done
> by Roland for ELF coredumps.  The branch is in
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.fdpic
> and it's based on top of #regset.base there (just the introduction of
> regset_get() wrapper for ->get(); nothing else from the regset series
> is needed).  Killing the old aout helpers is _not_ in this branch;
> followup cleanups live separately.
>
> 	First we need to sort out the mess with struct elf_prstatus,
> though.  It's used both for ELF and ELF_FDPIC coredumps, and it
> contains a couple of fields under ifdef on CONFIG_BINFMT_ELF_FDPIC.
> ELF is MMU-dependent and most, but not all configs that allow ELF_FDPIC
> are non-MMU.  ARM is an exception - there ELF_FDPIC is allowed both for
> MMU and non-MMU configs.  That's a problem - struct elf_prstatus is a
> part of coredump layout, so ELF coredumps produced by arm kernels that
> have ELF_FDPIC enabled are incompatible with those that have it disabled.
>
> 	The obvious solution is to introduce struct elf_prstatus_fdpic
> and use that in binfmt_elf_fdpic.c, taking these fields out of the
> normal struct elf_prstatus.  Unfortunately, the damn thing is defined in
> include/uapi/linux/elfcore.h, so nominally it's a part of userland ABI.
> However, not a single userland program actually includes linux/elfcore.h.
> The reason is that the definition in there uses elf_gregset_t as a member,
> and _that_ is not defined anywhere in the exported headers.  It is defined
> in (libc) sys/procfs.h, but the same file defines struct elf_prstatus
> as well.  So if you try to include linux/elfcore.h without having already
> pulled sys/procfs.h, it'll break on incomplete type of a member.  And if
> you have pulled sys/procfs.h, it'll break on redefining a structure.
> IOW, it's not usable and it never had been; as the matter of fact,
> that's the reason sys/procfs.h had been introduced back in 1996.
>
> 1/7) unexport linux/elfcore.h
> 	Takes it out of include/uapi/linux and moves the stuff that used
> to live there into include/linux/elfcore.h
>
> 2/7) take fdpic-related parts of elf_prstatus out
> 	Now we can take that ifdef out of the definition of elf_prstatus
> (as well as compat_elf_prstatus) and put the variant with those extra
> fields into binfmt_elf_fdpic.c, calling it elf_prstatus_fdpic there.
>
> 3/7) kill elf_fpxregs_t
> 	All code dealing with it (both in elf_fdpic and non-regset side
> of elf) is conditional upon ELF_CORE_COPY_XFPREGS.  And no architectures
> define that anymore.  Take the dead code out.
>
> 4/7) [elf-fdpic] coredump: don't bother with cyclic list for per-thread
> objects
> 5/7) [elf-fdpic] move allocation of elf_thread_status into
> elf_dump_thread_status()
> 6/7) [elf-fdpic] use elf_dump_thread_status() for the dumper thread as well
> 	Massaging fdpic coredump logics towards the regset side of
> elf coredump.
>
> 7/7) [elf-fdpic] switch coredump to regsets
> 	... and now we can switch from elf_core_copy_task_{,fp}regs()
> to regset_get().

I just did a quick read through.

The KABI bits look sane, or rather pulling definitions out of the KABI
headers because they are not usable seems like a reasonable response to
a messed up situation.  In the long run it would be good if we could get
some proper KABI headers for the format of coredumps.

I am a bit confused about what is happening in the cleanups, and frankly
the fault really lies with the binfmt_elf.c.  As binfmt_elf.c in Linus's
tree still has a regset and a non-regset version of core dumping.

What I see happening is that you are transforming what started off
as a copy of the non-regset version of elf coredumping and transforming
it into something close to the regset version of coredumping.  Which is
sensible.  The fact that the elf_fdpic code continues to use the
non-regset names for the functions it calls, and does not synchronize
it's structure with the ordinary elf core dumping code may be sensible
but it is extremely confusing to follow.

As a follow up it would probably good to sort out synchronize the
elf and elf_fdpic coredumping code as much as possible, just to simplify
future maintenance.

So for as much as I could understand and verify.

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

Eric

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-07-14 17:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-30  4:36 [RFC][PATCHES] converting FDPIC coredumps to regsets Al Viro
2020-06-30  4:41 ` [PATCH 1/7] unexport linux/elfcore.h Al Viro
2020-06-30  4:41   ` [PATCH 2/7] take fdpic-related parts of elf_prstatus out Al Viro
2020-06-30  4:41   ` [PATCH 3/7] kill elf_fpxregs_t Al Viro
2020-06-30  4:41   ` [PATCH 4/7] [elf-fdpic] coredump: don't bother with cyclic list for per-thread objects Al Viro
2020-06-30  4:41   ` [PATCH 5/7] [elf-fdpic] move allocation of elf_thread_status into elf_dump_thread_status() Al Viro
2020-06-30  4:41   ` [PATCH 6/7] [elf-fdpic] use elf_dump_thread_status() for the dumper thread as well Al Viro
2020-06-30  4:41   ` [PATCH 7/7] [elf-fdpic] switch coredump to regsets Al Viro
2020-06-30 15:45 ` [RFC][PATCHES] converting FDPIC coredumps " Nicolas Pitre
2020-07-14 17:14 ` Eric W. Biederman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).