After finding a bug in glibc the question came up how linux/elfcore.h
is supposed to be used from user space. As far as I can tell, it's
not possible, as it references data types that are simply unavailable
there.
The #ifndef __KERNEL__ section in that header dates back to when the
file was introduced in linux-1.3.5, and presumably was meant to
provide the structures for the libc sys/procfs.h implementation.
However, this was never portable to architectures other than x86-32,
and has been broken on that architecture at a later point.
These are the steps that I needed to make it possible to include the
header file, e.g. for libc self-testing in order to make sure the
structures are compatible with its own:
- drop the #ifndef __KERNEL__ section that are obviously useless
and get in the way
- change the pid_t references to __kernel_pid_t
- Move required data from the private x86 asm/elf.h file into
a new uapi/asm/elf.h. Some other architectures already do that,
but most of them do not. Before applying the patch, we have
to do this for all architectures
- Change ELF_NGREG to an integer literal constant instead of
a sizeof operation based on a private type.
Cc: Joseph Myers <[email protected]>
Cc: David Howells <[email protected]>
Cc: [email protected]
Link: https://patchwork.ozlabs.org/patch/969540/
Signed-off-by: Arnd Bergmann <[email protected]>
---
arch/x86/include/asm/elf.h | 24 +-----------------------
arch/x86/include/uapi/asm/elf.h | 30 ++++++++++++++++++++++++++++++
arch/x86/include/uapi/asm/signal.h | 2 +-
include/uapi/linux/elf.h | 1 +
include/uapi/linux/elfcore.h | 26 +++++---------------------
5 files changed, 38 insertions(+), 45 deletions(-)
create mode 100644 arch/x86/include/uapi/asm/elf.h
diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 0d157d2a1e2a..973bd7b5b164 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -10,18 +10,10 @@
#include <asm/ptrace.h>
#include <asm/user.h>
#include <asm/auxvec.h>
-
-typedef unsigned long elf_greg_t;
-
-#define ELF_NGREG (sizeof(struct user_regs_struct) / sizeof(elf_greg_t))
-typedef elf_greg_t elf_gregset_t[ELF_NGREG];
-
-typedef struct user_i387_struct elf_fpregset_t;
+#include <uapi/asm/elf.h>
#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
@@ -35,13 +27,6 @@ typedef struct user_fxsr_struct elf_fpxregset_t;
#define R_386_GOTPC 10
#define R_386_NUM 11
-/*
- * These are used to set parameters in the core dumps.
- */
-#define ELF_CLASS ELFCLASS32
-#define ELF_DATA ELFDATA2LSB
-#define ELF_ARCH EM_386
-
#else
/* x86-64 relocation types */
@@ -65,13 +50,6 @@ typedef struct user_fxsr_struct elf_fpxregset_t;
#define R_X86_64_NUM 16
-/*
- * These are used to set parameters in the core dumps.
- */
-#define ELF_CLASS ELFCLASS64
-#define ELF_DATA ELFDATA2LSB
-#define ELF_ARCH EM_X86_64
-
#endif
#include <asm/vdso.h>
diff --git a/arch/x86/include/uapi/asm/elf.h b/arch/x86/include/uapi/asm/elf.h
new file mode 100644
index 000000000000..a640e1224939
--- /dev/null
+++ b/arch/x86/include/uapi/asm/elf.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_ASM_X86_ELF_H
+#define _UAPI_ASM_X86_ELF_H
+
+#ifdef __i386__
+
+/*
+ * These are used to set parameters in the core dumps.
+ */
+#define ELF_CLASS ELFCLASS32
+#define ELF_DATA ELFDATA2LSB
+#define ELF_ARCH EM_386
+#define ELF_NGREG 17
+
+#else
+
+/*
+ * These are used to set parameters in the core dumps.
+ */
+#define ELF_CLASS ELFCLASS64
+#define ELF_DATA ELFDATA2LSB
+#define ELF_ARCH EM_X86_64
+#define ELF_NGREG 27
+
+#endif /* __i386__ */
+
+typedef unsigned long elf_greg_t;
+typedef elf_greg_t elf_gregset_t[ELF_NGREG];
+
+#endif
diff --git a/arch/x86/include/uapi/asm/signal.h b/arch/x86/include/uapi/asm/signal.h
index e5745d593dc7..00f273eaddf7 100644
--- a/arch/x86/include/uapi/asm/signal.h
+++ b/arch/x86/include/uapi/asm/signal.h
@@ -128,7 +128,7 @@ struct sigaction {
typedef struct sigaltstack {
void __user *ss_sp;
int ss_flags;
- size_t ss_size;
+ __kernel_size_t ss_size;
} stack_t;
#endif /* __ASSEMBLY__ */
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index c5358e0ae7c5..e1e4561ed9c2 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -4,6 +4,7 @@
#include <linux/types.h>
#include <linux/elf-em.h>
+#include <asm/elf.h>
/* 32-bit ELF base types. */
typedef __u32 Elf32_Addr;
diff --git a/include/uapi/linux/elfcore.h b/include/uapi/linux/elfcore.h
index baf03562306d..9c0078004275 100644
--- a/include/uapi/linux/elfcore.h
+++ b/include/uapi/linux/elfcore.h
@@ -16,15 +16,6 @@ struct elf_siginfo
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_"
@@ -49,10 +40,10 @@ struct elf_prstatus
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;
+ __kernel_pid_t pr_pid;
+ __kernel_pid_t pr_ppid;
+ __kernel_pid_t pr_pgrp;
+ __kernel_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 */
@@ -85,17 +76,10 @@ struct elf_prpsinfo
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;
+ __kernel_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 */
--
2.18.0
* Arnd Bergmann <[email protected]> wrote:
> After finding a bug in glibc the question came up how linux/elfcore.h
> is supposed to be used from user space. As far as I can tell, it's
> not possible, as it references data types that are simply unavailable
> there.
>
> The #ifndef __KERNEL__ section in that header dates back to when the
> file was introduced in linux-1.3.5, and presumably was meant to
> provide the structures for the libc sys/procfs.h implementation.
> However, this was never portable to architectures other than x86-32,
> and has been broken on that architecture at a later point.
>
> These are the steps that I needed to make it possible to include the
> header file, e.g. for libc self-testing in order to make sure the
> structures are compatible with its own:
>
> - drop the #ifndef __KERNEL__ section that are obviously useless
> and get in the way
>
> - change the pid_t references to __kernel_pid_t
>
> - Move required data from the private x86 asm/elf.h file into
> a new uapi/asm/elf.h. Some other architectures already do that,
> but most of them do not. Before applying the patch, we have
> to do this for all architectures
>
> - Change ELF_NGREG to an integer literal constant instead of
> a sizeof operation based on a private type.
>
> Cc: Joseph Myers?<[email protected]>
> Cc: David Howells <[email protected]>
> Cc: [email protected]
> Link: https://patchwork.ozlabs.org/patch/969540/
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> arch/x86/include/asm/elf.h | 24 +-----------------------
> arch/x86/include/uapi/asm/elf.h | 30 ++++++++++++++++++++++++++++++
> arch/x86/include/uapi/asm/signal.h | 2 +-
> include/uapi/linux/elf.h | 1 +
> include/uapi/linux/elfcore.h | 26 +++++---------------------
> 5 files changed, 38 insertions(+), 45 deletions(-)
> create mode 100644 arch/x86/include/uapi/asm/elf.h
Acked-by: Ingo Molnar <[email protected]>
I suspect this wants to go through -mm, or do you want to carry it?
Thanks,
Ingo
On Fri, Sep 14, 2018 at 7:45 PM Ingo Molnar <[email protected]> wrote:
> * Arnd Bergmann <[email protected]> wrote:
> > - Move required data from the private x86 asm/elf.h file into
> > a new uapi/asm/elf.h. Some other architectures already do that,
> > but most of them do not. Before applying the patch, we have
> > to do this for all architectures
>
> Acked-by: Ingo Molnar <[email protected]>
Thanks,
> I suspect this wants to go through -mm, or do you want to carry it?
-mm is probably best. For now, I just want to see if there are any
concerns about this, and then I'd have to do the full version that
changes all architectures.
Arnd
* Arnd Bergmann <[email protected]> wrote:
> diff --git a/arch/x86/include/uapi/asm/elf.h b/arch/x86/include/uapi/asm/elf.h
> new file mode 100644
> index 000000000000..a640e1224939
> --- /dev/null
> +++ b/arch/x86/include/uapi/asm/elf.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI_ASM_X86_ELF_H
> +#define _UAPI_ASM_X86_ELF_H
> +
> +#ifdef __i386__
> +
> +/*
> + * These are used to set parameters in the core dumps.
> + */
> +#define ELF_CLASS ELFCLASS32
> +#define ELF_DATA ELFDATA2LSB
> +#define ELF_ARCH EM_386
> +#define ELF_NGREG 17
> +
> +#else
> +
> +/*
> + * These are used to set parameters in the core dumps.
> + */
> +#define ELF_CLASS ELFCLASS64
> +#define ELF_DATA ELFDATA2LSB
> +#define ELF_ARCH EM_X86_64
> +#define ELF_NGREG 27
> +
> +#endif /* __i386__ */
> +
> +typedef unsigned long elf_greg_t;
> +typedef elf_greg_t elf_gregset_t[ELF_NGREG];
> +
> +#endif
On a second thought, maybe deduplicate the comments? Something like:
/*
* These are used to set parameters in core dumps:
*/
#ifdef __i386__
# define ELF_CLASS ELFCLASS32
# define ELF_DATA ELFDATA2LSB
# define ELF_ARCH EM_386
# define ELF_NGREG 17
#else
# define ELF_CLASS ELFCLASS64
# define ELF_DATA ELFDATA2LSB
# define ELF_ARCH EM_X86_64
# define ELF_NGREG 27
#endif
Note:
- I fixed a typo in the comment.
- Aligned the blocks vertically for better visibility.
- The closing #endif comment became unnecessary as well, due to the much more obvious
structure when written this way.
The type changes/cleanups look good otherwise: it's quite probable that it was never directly
included in any user-space library in any sane fashion before, so it's not really an UAPI that
was relied on, as long as it doesn't break the build anywhere.
Thanks,
Ingo
On Fri, 14 Sep 2018, Arnd Bergmann wrote:
> +typedef unsigned long elf_greg_t;
Does that need to be unsigned long long for x32? (At least glibc thinks
so; I've not tested what an x32 core dump actually looks like, but to be
useful it certainly ought to have 64-bit registers.)
--
Joseph S. Myers
[email protected]
On Mon, Sep 17, 2018 at 5:05 AM Joseph Myers <[email protected]> wrote:
>
> On Fri, 14 Sep 2018, Arnd Bergmann wrote:
>
> > +typedef unsigned long elf_greg_t;
>
> Does that need to be unsigned long long for x32? (At least glibc thinks
> so; I've not tested what an x32 core dump actually looks like, but to be
> useful it certainly ought to have 64-bit registers.)
Yes, I think that's right. 'unsigned long' was correct inside of the kernel,
but copying it into a uapi header means we have to use '__kernel_ulong_t'
so it gets interpreted right by x32 user space.
Arnd