Message ID | 20180914113929.953895-1-arnd@arndb.de |
---|---|
State | New |
Headers | show |
Series | [RFC] making uapi/linux/elfcore.h useful again | expand |
* Arnd Bergmann <arnd@arndb.de> 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 <joseph@codesourcery.com> > Cc: David Howells <dhowells@redhat.com> > Cc: libc-alpha@sourceware.org > Link: https://patchwork.ozlabs.org/patch/969540/ > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > 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 <mingo@kernel.org> 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 <mingo@kernel.org> wrote: > * Arnd Bergmann <arnd@arndb.de> 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 <mingo@kernel.org> 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 <arnd@arndb.de> 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
joseph@codesourcery.com
On Mon, Sep 17, 2018 at 5:05 AM Joseph Myers <joseph@codesourcery.com> 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
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 */
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 <joseph@codesourcery.com> Cc: David Howells <dhowells@redhat.com> Cc: libc-alpha@sourceware.org Link: https://patchwork.ozlabs.org/patch/969540/ Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- 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 -- 2.18.0