Partly revert commit 3316ec8ccd34 ("x86/elf: Use e_machine to check for
x32/ia32 in setup_additional_pages()") and commit 9a29a671902c ("elf:
Expose ELF header on arch_setup_additional_pages()".
Both patches did a good thing: removed usage of TIF_X32, but with
a price of additional macros ARCH_SETUP_ADDITIONAL_PAGES() and ifdeffs.
Instead, use in_x32_syscall() - the first thing load_elf_binary() does
after parsing and checking new ELF binary. It's done that early after
exec() that mmap() code already uses it straight away, which is needed
to know which mmap_base to use (see arch_pick_mmap_layout()).
Add comments to describe how it works.
Cc: [email protected]
Signed-off-by: Dmitry Safonov <[email protected]>
---
arch/x86/entry/vdso/vma.c | 4 ++--
arch/x86/include/asm/compat.h | 6 ++++++
arch/x86/include/asm/elf.h | 6 ++----
fs/binfmt_elf.c | 10 +++++++---
fs/compat_binfmt_elf.c | 11 +++--------
include/linux/elf.h | 5 -----
6 files changed, 20 insertions(+), 22 deletions(-)
diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
index 235a5794296a..43d42ce82e86 100644
--- a/arch/x86/entry/vdso/vma.c
+++ b/arch/x86/entry/vdso/vma.c
@@ -396,10 +396,10 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
#ifdef CONFIG_COMPAT
int compat_arch_setup_additional_pages(struct linux_binprm *bprm,
- int uses_interp, bool x32)
+ int uses_interp)
{
#ifdef CONFIG_X86_X32_ABI
- if (x32) {
+ if (in_x32_syscall()) {
if (!vdso64_enabled)
return 0;
return map_vdso_randomized(&vdso_image_x32);
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index be09c7eac89f..58f76665042c 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -186,6 +186,12 @@ static inline bool in_x32_syscall(void)
return false;
}
+/*
+ * Valid all the time on the context of a process that performs syscall.
+ * Just exec()ed process has __X32_SYSCALL_BIT or TS_COMPAT set very
+ * early in load_binary() on setting personality and flags.
+ * See also set_personality_ia32().
+ */
static inline bool in_32bit_syscall(void)
{
return in_ia32_syscall() || in_x32_syscall();
diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 7d7500806af8..c0b5733005af 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -383,10 +383,8 @@ struct linux_binprm;
extern int arch_setup_additional_pages(struct linux_binprm *bprm,
int uses_interp);
extern int compat_arch_setup_additional_pages(struct linux_binprm *bprm,
- int uses_interp, bool x32);
-#define COMPAT_ARCH_SETUP_ADDITIONAL_PAGES(bprm, ex, interpreter) \
- compat_arch_setup_additional_pages(bprm, interpreter, \
- (ex->e_machine == EM_X86_64))
+ int uses_interp);
+#define compat_arch_setup_additional_pages compat_arch_setup_additional_pages
extern bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs);
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 187b3f2b9202..2347d9067df6 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1002,8 +1002,12 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (retval)
goto out_free_dentry;
- /* Do this immediately, since STACK_TOP as used in setup_arg_pages
- may depend on the personality. */
+ /*
+ * Do this immediately, since STACK_TOP as used in setup_arg_pages
+ * may depend on the personality. At this moment sys_exec()
+ * for loaded compatible application becomes non-native syscall
+ * and in_compat_syscall() starts working.
+ */
SET_PERSONALITY2(*elf_ex, &arch_state);
if (elf_read_implies_exec(*elf_ex, executable_stack))
current->personality |= READ_IMPLIES_EXEC;
@@ -1249,7 +1253,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
set_binfmt(&elf_format);
#ifdef ARCH_HAS_SETUP_ADDITIONAL_PAGES
- retval = ARCH_SETUP_ADDITIONAL_PAGES(bprm, elf_ex, !!interpreter);
+ retval = arch_setup_additional_pages(bprm, !!interpreter);
if (retval < 0)
goto out;
#endif /* ARCH_HAS_SETUP_ADDITIONAL_PAGES */
diff --git a/fs/compat_binfmt_elf.c b/fs/compat_binfmt_elf.c
index 95e72d271b95..049ba7c011b9 100644
--- a/fs/compat_binfmt_elf.c
+++ b/fs/compat_binfmt_elf.c
@@ -110,16 +110,11 @@
#define START_THREAD COMPAT_START_THREAD
#endif
-#ifdef compat_arch_setup_additional_pages
-#define COMPAT_ARCH_SETUP_ADDITIONAL_PAGES(bprm, ex, interpreter) \
- compat_arch_setup_additional_pages(bprm, interpreter)
-#endif
-
-#ifdef COMPAT_ARCH_SETUP_ADDITIONAL_PAGES
+#ifdef compat_arch_setup_additional_pages
#undef ARCH_HAS_SETUP_ADDITIONAL_PAGES
#define ARCH_HAS_SETUP_ADDITIONAL_PAGES 1
-#undef ARCH_SETUP_ADDITIONAL_PAGES
-#define ARCH_SETUP_ADDITIONAL_PAGES COMPAT_ARCH_SETUP_ADDITIONAL_PAGES
+#undef arch_setup_additional_pages
+#define arch_setup_additional_pages compat_arch_setup_additional_pages
#endif
#ifdef compat_elf_read_implies_exec
diff --git a/include/linux/elf.h b/include/linux/elf.h
index c9a46c4e183b..6dbcfe7a3fd7 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -27,11 +27,6 @@
start_thread(regs, elf_entry, start_stack)
#endif
-#if defined(ARCH_HAS_SETUP_ADDITIONAL_PAGES) && !defined(ARCH_SETUP_ADDITIONAL_PAGES)
-#define ARCH_SETUP_ADDITIONAL_PAGES(bprm, ex, interpreter) \
- arch_setup_additional_pages(bprm, interpreter)
-#endif
-
#define ELF32_GNU_PROPERTY_ALIGN 4
#define ELF64_GNU_PROPERTY_ALIGN 8
--
2.31.1
On Fri, Jun 11 2021 at 19:02, Dmitry Safonov wrote:
> Partly revert commit 3316ec8ccd34 ("x86/elf: Use e_machine to check for
> x32/ia32 in setup_additional_pages()") and commit 9a29a671902c ("elf:
> Expose ELF header on arch_setup_additional_pages()".
> Both patches did a good thing: removed usage of TIF_X32, but with
> a price of additional macros ARCH_SETUP_ADDITIONAL_PAGES() and ifdeffs.
>
> Instead, use in_x32_syscall() - the first thing load_elf_binary() does
> after parsing and checking new ELF binary. It's done that early after
> exec() that mmap() code already uses it straight away, which is needed
> to know which mmap_base to use (see arch_pick_mmap_layout()).
> Add comments to describe how it works.
I still have no idea what this is trying to solve. All you describe is
what this does.
Thanks,
tglx
Hi Thomas,
On 6/19/21 9:41 PM, Thomas Gleixner wrote:
> On Fri, Jun 11 2021 at 19:02, Dmitry Safonov wrote:
>> Partly revert commit 3316ec8ccd34 ("x86/elf: Use e_machine to check for
>> x32/ia32 in setup_additional_pages()") and commit 9a29a671902c ("elf:
>> Expose ELF header on arch_setup_additional_pages()".
>> Both patches did a good thing: removed usage of TIF_X32, but with
>> a price of additional macros ARCH_SETUP_ADDITIONAL_PAGES() and ifdeffs.
>>
>> Instead, use in_x32_syscall() - the first thing load_elf_binary() does
>> after parsing and checking new ELF binary. It's done that early after
>> exec() that mmap() code already uses it straight away, which is needed
>> to know which mmap_base to use (see arch_pick_mmap_layout()).
>> Add comments to describe how it works.
>
> I still have no idea what this is trying to solve. All you describe is
> what this does.
Yeah, I still have problems with describing why rather than what sometimes..
How about something like this:
In order to simplify code, new macro ARCH_SETUP_ADDITIONAL_PAGES() can
be dropped as well as a new argument in
compat_arch_setup_additional_pages(). Resulting in partial revert of the
commit 3316ec8ccd34 ("x86/elf: Use e_machine to check for x32/ia32 in
setup_additional_pages()") and the commit 9a29a671902c ("elf: Expose ELF
header on arch_setup_additional_pages()".
Instead of a new macro and an additional function argument, use
in_x32_syscall() which is already valid at the moment of premapping
vDSO. Add comments to describe how it works.
Thanks for reviewing,
Dmitry