We are running out of TI flags for x86. This patchset removes several
usages of TIF_IA32 and TIF_x32 in preparation to reclaim these flags.
After these cleanups, there is still one more user for both of them,
which I need to take a better look before removing.
Many of the ideas for this patchset came from Andy Lutomirski (Thank
you!)
These were tested by exercising these paths with x32 and ia32 binaries.
Gabriel Krisman Bertazi (6):
x86: events: Avoid TIF_IA32 when checking 64bit mode
x86: Simplify compat syscall userspace allocation
x86: oprofile: Avoid TIF_IA32 when checking 64bit mode
x86: elf: Use e_machine to choose DLINFO in compat
x86: elf: Use e_machine to select start_thread for x32
x86: elf: Use e_machine to select setup_additional_pages for x32
arch/x86/entry/vdso/vma.c | 21 ++++++++++++---------
arch/x86/events/core.c | 2 +-
arch/x86/events/intel/ds.c | 2 +-
arch/x86/events/intel/lbr.c | 2 +-
arch/x86/include/asm/compat.h | 15 +++++++--------
arch/x86/include/asm/elf.h | 24 ++++++++++++++++++------
arch/x86/kernel/perf_regs.c | 2 +-
arch/x86/kernel/process_64.c | 11 +++++++----
arch/x86/oprofile/backtrace.c | 2 +-
9 files changed, 49 insertions(+), 32 deletions(-)
--
2.28.0
In preparation to remove TIF_IA32, stop using it in perf events code.
Tested by running perf on 32-bit, 64-bit and x32 applications.
Suggested-by: Andy Lutomirski <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
arch/x86/events/core.c | 2 +-
arch/x86/events/intel/ds.c | 2 +-
arch/x86/events/intel/lbr.c | 2 +-
arch/x86/kernel/perf_regs.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 1cbf57dc2ac8..4fe82d9d959b 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2499,7 +2499,7 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *ent
struct stack_frame_ia32 frame;
const struct stack_frame_ia32 __user *fp;
- if (!test_thread_flag(TIF_IA32))
+ if (user_64bit_mode(regs))
return 0;
cs_base = get_segment_base(regs->cs);
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index 86848c57b55e..94bd0d3acd15 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -1261,7 +1261,7 @@ static int intel_pmu_pebs_fixup_ip(struct pt_regs *regs)
old_to = to;
#ifdef CONFIG_X86_64
- is_64bit = kernel_ip(to) || !test_thread_flag(TIF_IA32);
+ is_64bit = kernel_ip(to) || any_64bit_mode(regs);
#endif
insn_init(&insn, kaddr, size, is_64bit);
insn_get_length(&insn);
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 8961653c5dd2..1aadb253d296 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1221,7 +1221,7 @@ static int branch_type(unsigned long from, unsigned long to, int abort)
* on 64-bit systems running 32-bit apps
*/
#ifdef CONFIG_X86_64
- is64 = kernel_ip((unsigned long)addr) || !test_thread_flag(TIF_IA32);
+ is64 = kernel_ip((unsigned long)addr) || any_64bit_mode(current_pt_regs());
#endif
insn_init(&insn, addr, bytes_read, is64);
insn_get_opcode(&insn);
diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
index bb7e1132290b..9332c49a64a8 100644
--- a/arch/x86/kernel/perf_regs.c
+++ b/arch/x86/kernel/perf_regs.c
@@ -123,7 +123,7 @@ int perf_reg_validate(u64 mask)
u64 perf_reg_abi(struct task_struct *task)
{
- if (test_tsk_thread_flag(task, TIF_IA32))
+ if (!user_64bit_mode(task_pt_regs(task)))
return PERF_SAMPLE_REGS_ABI_32;
else
return PERF_SAMPLE_REGS_ABI_64;
--
2.28.0
In preparation to remove TIF_IA32, stop using it in oprofile code.
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
arch/x86/oprofile/backtrace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/oprofile/backtrace.c b/arch/x86/oprofile/backtrace.c
index a2488b6e27d6..1d8391fcca68 100644
--- a/arch/x86/oprofile/backtrace.c
+++ b/arch/x86/oprofile/backtrace.c
@@ -49,7 +49,7 @@ x86_backtrace_32(struct pt_regs * const regs, unsigned int depth)
struct stack_frame_ia32 *head;
/* User process is IA32 */
- if (!current || !test_thread_flag(TIF_IA32))
+ if (!current || user_64bit_mode(regs))
return 0;
head = (struct stack_frame_ia32 *) regs->bp;
--
2.28.0
Since TIF_X32 is going away, avoid using it to find the ELF type on
ARCH_DLINFO.
According to SysV AMD64 ABI Draft, an AMD64 ELF object using ILP32 must
have ELFCLASS32 with (E_MACHINE == EM_X86_64), so use that ELF field to
differentiate a x32 object from a IA32 object when loading ARCH_DLINFO
in compat mode.
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
arch/x86/include/asm/elf.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index b9a5d488f1a5..9220efc65d78 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -361,7 +361,7 @@ do { \
#define AT_SYSINFO 32
#define COMPAT_ARCH_DLINFO \
-if (test_thread_flag(TIF_X32)) \
+if (exec->e_machine == EM_X86_64) \
ARCH_DLINFO_X32; \
else \
ARCH_DLINFO_IA32
--
2.28.0
Since TIF_X32 is going away, avoid using it to find the ELF type when
choosing which additional pages to set up.
According to SysV AMD64 ABI Draft, an AMD64 ELF object using ILP32 must
have ELFCLASS32 with (E_MACHINE == EM_X86_64), so use that ELF field to
differentiate a x32 object from a IA32 object when executing
start_thread in compat mode.
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
arch/x86/entry/vdso/vma.c | 21 ++++++++++++---------
arch/x86/include/asm/elf.h | 11 ++++++++---
2 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
index 9185cb1d13b9..7a3cda8294a3 100644
--- a/arch/x86/entry/vdso/vma.c
+++ b/arch/x86/entry/vdso/vma.c
@@ -412,22 +412,25 @@ 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)
+int compat_arch_setup_additional_pages_ia32(struct linux_binprm *bprm,
+ int uses_interp)
{
-#ifdef CONFIG_X86_X32_ABI
- if (test_thread_flag(TIF_X32)) {
- if (!vdso64_enabled)
- return 0;
- return map_vdso_randomized(&vdso_image_x32);
- }
-#endif
#ifdef CONFIG_IA32_EMULATION
return load_vdso32();
#else
return 0;
#endif
}
+
+int compat_arch_setup_additional_pages_x32(struct linux_binprm *bprm,
+ int uses_interp)
+{
+#ifdef CONFIG_X86_X32_ABI
+ if (vdso64_enabled)
+ return map_vdso_randomized(&vdso_image_x32);
+#endif
+ return 0;
+}
#endif
#else
int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 33c1c9be2e07..4d91f5b1079f 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -388,9 +388,14 @@ struct linux_binprm;
#define ARCH_HAS_SETUP_ADDITIONAL_PAGES 1
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);
-#define compat_arch_setup_additional_pages compat_arch_setup_additional_pages
+extern int compat_arch_setup_additional_pages_ia32(struct linux_binprm *bprm,
+ int uses_interp);
+extern int compat_arch_setup_additional_pages_x32(struct linux_binprm *bprm,
+ int uses_interp);
+
+#define compat_arch_setup_additional_pages \
+ ((elf_ex->e_machine == EM_X86_64) ? \
+ compat_arch_setup_additional_pages_x32 : compat_arch_setup_additional_pages_ia32)
/* Do not change the values. See get_align_mask() */
enum align_flags {
--
2.28.0
Since TIF_X32 is going away, avoid using it to find the ELF type on
compat_start_thread
According to SysV AMD64 ABI Draft, an AMD64 ELF object using ILP32 must
have ELFCLASS32 with (E_MACHINE == EM_X86_64), so use that ELF field to
differentiate a x32 object from a IA32 object when executing
start_thread in compat mode.
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
arch/x86/include/asm/elf.h | 11 +++++++++--
arch/x86/kernel/process_64.c | 11 +++++++----
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 9220efc65d78..33c1c9be2e07 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -186,8 +186,15 @@ static inline void elf_common_init(struct thread_struct *t,
#define COMPAT_ELF_PLAT_INIT(regs, load_addr) \
elf_common_init(¤t->thread, regs, __USER_DS)
-void compat_start_thread(struct pt_regs *regs, u32 new_ip, u32 new_sp);
-#define compat_start_thread compat_start_thread
+void compat_start_thread_ia32(struct pt_regs *regs, u32 new_ip, u32 new_sp);
+void compat_start_thread_x32(struct pt_regs *regs, u32 new_ip, u32 new_sp);
+#define compat_start_thread(regs, new_ip, new_sp) \
+do { \
+ if (elf_ex->e_machine == EM_X86_64) \
+ compat_start_thread_x32(regs, new_ip, new_sp); \
+ else \
+ compat_start_thread_ia32(regs, new_ip, new_sp); \
+} while (0)
void set_personality_ia32(bool);
#define COMPAT_SET_PERSONALITY(ex) \
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 9afefe325acb..56e882c339e6 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -511,12 +511,15 @@ start_thread(struct pt_regs *regs, unsigned long new_ip, unsigned long new_sp)
EXPORT_SYMBOL_GPL(start_thread);
#ifdef CONFIG_COMPAT
-void compat_start_thread(struct pt_regs *regs, u32 new_ip, u32 new_sp)
+void compat_start_thread_ia32(struct pt_regs *regs, u32 new_ip, u32 new_sp)
{
start_thread_common(regs, new_ip, new_sp,
- test_thread_flag(TIF_X32)
- ? __USER_CS : __USER32_CS,
- __USER_DS, __USER_DS);
+ __USER32_CS, __USER_DS, __USER_DS);
+}
+void compat_start_thread_x32(struct pt_regs *regs, u32 new_ip, u32 new_sp)
+{
+ start_thread_common(regs, new_ip, new_sp,
+ __USER_CS, __USER_DS, __USER_DS);
}
#endif
--
2.28.0
When allocating user memory space for a compat system call, don't
consider whether the originating code is IA32 or X32, just allocate from
a safe region for both, beyond the redzone. This should be safe for
IA32, and has the benefit of avoiding TIF_IA32, which we want to drop.
Suggested-by: Andy Lutomirski <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
arch/x86/include/asm/compat.h | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index d4edf281fff4..a4b5126dff4e 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -179,14 +179,13 @@ typedef struct user_regs_struct compat_elf_gregset_t;
static inline void __user *arch_compat_alloc_user_space(long len)
{
- compat_uptr_t sp;
-
- if (test_thread_flag(TIF_IA32)) {
- sp = task_pt_regs(current)->sp;
- } else {
- /* -128 for the x32 ABI redzone */
- sp = task_pt_regs(current)->sp - 128;
- }
+ compat_uptr_t sp = task_pt_regs(current)->sp;
+
+ /*
+ * -128 for the x32 ABI redzone. For IA32, it is not strictly
+ * necessary, but not harmful.
+ */
+ sp -= 128;
return (void __user *)round_down(sp - len, 16);
}
--
2.28.0
On Sat, Sep 12, 2020 at 03:05:49AM -0400, Gabriel Krisman Bertazi wrote:
> When allocating user memory space for a compat system call, don't
> consider whether the originating code is IA32 or X32, just allocate from
> a safe region for both, beyond the redzone. This should be safe for
> IA32, and has the benefit of avoiding TIF_IA32, which we want to drop.
This doesn't look wrong, by why bother (maybe Ccing me on the whole
seris as you always should instead of sending annoying out of context
single patches would have told..).
We will hopefully kill off compat_alloc_user_space in the next few
merge windows..
Christoph Hellwig <[email protected]> writes:
> On Sat, Sep 12, 2020 at 03:05:49AM -0400, Gabriel Krisman Bertazi wrote:
>> When allocating user memory space for a compat system call, don't
>> consider whether the originating code is IA32 or X32, just allocate from
>> a safe region for both, beyond the redzone. This should be safe for
>> IA32, and has the benefit of avoiding TIF_IA32, which we want to drop.
>
> This doesn't look wrong, by why bother (maybe Ccing me on the whole
> seris as you always should instead of sending annoying out of context
> single patches would have told..).
Hi Chris,
Thanks for the quick reply. sorry and I will make sure to cc you for the
rest of the series if this spin again. The reason is the removal of
TIF_IA32 to reclaim some bits in the ti flags.
If you want to see the rest of it immediately: <https://lkml.org/lkml/2020/9/12/28>
> We will hopefully kill off compat_alloc_user_space in the next few
> merge windows..
I plan to kill TIF_IA32 hopefully in the next merge window, to
facilitate other work I'm doing and I wouldn't like to wait for other
stuff, since this is trivial enough. Can I get your reviewed-by here?
Thanks,
--
Gabriel Krisman Bertazi
On Sat, Sep 12, 2020 at 03:05:48AM -0400, Gabriel Krisman Bertazi wrote:
> In preparation to remove TIF_IA32, stop using it in perf events code.
>
> Tested by running perf on 32-bit, 64-bit and x32 applications.
>
> Suggested-by: Andy Lutomirski <[email protected]>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>