Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1381722 > unrolled thread

[PATCHv5 1/3] x86: rename is_{ia32,x32}_task to in_{ia32,x32}_syscall

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCHv5 1/3] x86: rename is_{ia32,x32}_task to in_{ia32,x32}_syscall Dmitry Safonov <dsafonov@virtuozzo.com> - 2016-04-18 15:50 +0200
    [PATCHv5 3/3] selftest/x86: add mremap vdso 32-bit test Dmitry Safonov <dsafonov@virtuozzo.com> - 2016-04-18 15:50 +0200
      Re: [PATCHv5 3/3] selftest/x86: add mremap vdso 32-bit test Andy Lutomirski <luto@amacapital.net> - 2016-04-21 22:10 +0200
    [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to  in_{ia32,x32}_syscall() tip-bot for Dmitry Safonov <tipbot@zytor.com> - 2016-04-19 11:40 +0200
      Re: [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to  in_{ia32,x32}_syscall() Ingo Molnar <mingo@kernel.org> - 2016-04-19 13:20 +0200
        Re: [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to  in_{ia32,x32}_syscall() Borislav Petkov <bp@alien8.de> - 2016-04-19 13:40 +0200
          Re: [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall() "H. Peter Anvin" <hpa@zytor.com> - 2016-04-19 19:10 +0200
        Re: [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall() Andy Lutomirski <luto@amacapital.net> - 2016-04-19 18:10 +0200
          Re: [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to  in_{ia32,x32}_syscall() Ingo Molnar <mingo@kernel.org> - 2016-04-22 10:40 +0200

#1381722 — [PATCHv5 1/3] x86: rename is_{ia32,x32}_task to in_{ia32,x32}_syscall

Impact: clearify meaning

Suggested-by: Andy Lutomirski <luto@amacapital.net>
Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
Acked-by: Andy Lutomirski <luto@kernel.org>
---
v3: initial patch

 arch/x86/entry/common.c            | 2 +-
 arch/x86/include/asm/compat.h      | 4 ++--
 arch/x86/include/asm/thread_info.h | 2 +-
 arch/x86/kernel/process_64.c       | 2 +-
 arch/x86/kernel/ptrace.c           | 2 +-
 arch/x86/kernel/signal.c           | 2 +-
 arch/x86/kernel/uprobes.c          | 2 +-
 7 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index e79d93d44ecd..ec138e538c44 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -191,7 +191,7 @@ long syscall_trace_enter_phase2(struct pt_regs *regs, u32 arch,
 
 long syscall_trace_enter(struct pt_regs *regs)
 {
-	u32 arch = is_ia32_task() ? AUDIT_ARCH_I386 : AUDIT_ARCH_X86_64;
+	u32 arch = in_ia32_syscall() ? AUDIT_ARCH_I386 : AUDIT_ARCH_X86_64;
 	unsigned long phase1_result = syscall_trace_enter_phase1(regs, arch);
 
 	if (phase1_result == 0)
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index ebb102e1bbc7..5a3b2c119ed0 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -307,7 +307,7 @@ static inline void __user *arch_compat_alloc_user_space(long len)
 	return (void __user *)round_down(sp - len, 16);
 }
 
-static inline bool is_x32_task(void)
+static inline bool in_x32_syscall(void)
 {
 #ifdef CONFIG_X86_X32_ABI
 	if (task_pt_regs(current)->orig_ax & __X32_SYSCALL_BIT)
@@ -318,7 +318,7 @@ static inline bool is_x32_task(void)
 
 static inline bool in_compat_syscall(void)
 {
-	return is_ia32_task() || is_x32_task();
+	return in_ia32_syscall() || in_x32_syscall();
 }
 #define in_compat_syscall in_compat_syscall	/* override the generic impl */
 
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index ffae84df8a93..30c133ac05cd 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -255,7 +255,7 @@ static inline bool test_and_clear_restore_sigmask(void)
 	return true;
 }
 
-static inline bool is_ia32_task(void)
+static inline bool in_ia32_syscall(void)
 {
 #ifdef CONFIG_X86_32
 	return true;
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 6cbab31ac23a..4a62ec457b56 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -210,7 +210,7 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
 	 */
 	if (clone_flags & CLONE_SETTLS) {
 #ifdef CONFIG_IA32_EMULATION
-		if (is_ia32_task())
+		if (in_ia32_syscall())
 			err = do_set_thread_area(p, -1,
 				(struct user_desc __user *)tls, 0);
 		else
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 32e9d9cbb884..0f4d2a5df2dc 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1266,7 +1266,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
 			compat_ulong_t caddr, compat_ulong_t cdata)
 {
 #ifdef CONFIG_X86_X32_ABI
-	if (!is_ia32_task())
+	if (!in_ia32_syscall())
 		return x32_arch_ptrace(child, request, caddr, cdata);
 #endif
 #ifdef CONFIG_IA32_EMULATION
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 548ddf7d6fd2..aa31265aa61d 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -762,7 +762,7 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs)
 {
 #ifdef CONFIG_X86_64
-	if (is_ia32_task())
+	if (in_ia32_syscall())
 		return __NR_ia32_restart_syscall;
 #endif
 #ifdef CONFIG_X86_X32_ABI
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index bf4db6eaec8f..98b4dc87628b 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -516,7 +516,7 @@ struct uprobe_xol_ops {
 
 static inline int sizeof_long(void)
 {
-	return is_ia32_task() ? 4 : 8;
+	return in_ia32_syscall() ? 4 : 8;
 }
 
 static int default_pre_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs)
-- 
2.8.0

[toc] | [next] | [standalone]

#1381723 — [PATCHv5 3/3] selftest/x86: add mremap vdso 32-bit test

Should print on success:
[root@localhost ~]# ./test_mremap_vdso_32
	AT_SYSINFO_EHDR is 0xf773f000
[NOTE]	Moving vDSO: [f773f000, f7740000] -> [a000000, a001000]
[OK]
Or segfault if landing was bad (before patches):
[root@localhost ~]# ./test_mremap_vdso_32
	AT_SYSINFO_EHDR is 0xf774f000
[NOTE]	Moving vDSO: [f774f000, f7750000] -> [a000000, a001000]
Segmentation fault (core dumped)

Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: linux-kselftest@vger.kernel.org
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
---
v5: initial version

 tools/testing/selftests/x86/Makefile           |  2 +-
 tools/testing/selftests/x86/test_mremap_vdso.c | 72 ++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/x86/test_mremap_vdso.c

diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index b47ebd170690..c7162b511ab0 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -7,7 +7,7 @@ include ../lib.mk
 TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_syscall \
 			check_initial_reg_state sigreturn ldt_gdt iopl
 TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \
-			test_FCMOV test_FCOMI test_FISTTP \
+			test_FCMOV test_FCOMI test_FISTTP test_mremap_vdso \
 			vdso_restorer
 
 TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
diff --git a/tools/testing/selftests/x86/test_mremap_vdso.c b/tools/testing/selftests/x86/test_mremap_vdso.c
new file mode 100644
index 000000000000..a470790e2118
--- /dev/null
+++ b/tools/testing/selftests/x86/test_mremap_vdso.c
@@ -0,0 +1,72 @@
+/*
+ * 32-bit test to check vdso mremap.
+ *
+ * Copyright (c) 2016 Dmitry Safonov
+ * Suggested-by: Andrew Lutomirski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+/*
+ * Can be built statically:
+ * gcc -Os -Wall -static -m32 test_mremap_vdso.c
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+
+#include <sys/mman.h>
+#include <sys/auxv.h>
+#include <sys/syscall.h>
+
+#if !defined(__i386__)
+int main(int argc, char **argv, char **envp)
+{
+	printf("[SKIP]\tNot a 32-bit x86 userspace\n");
+	return 0;
+}
+#else
+
+#define PAGE_SIZE	4096
+#define VDSO_SIZE	PAGE_SIZE
+
+int main(int argc, char **argv, char **envp)
+{
+	unsigned long vdso_addr, dest_addr;
+	void *new_addr;
+	const char *ok_string = "[OK]\n";
+
+	vdso_addr = getauxval(AT_SYSINFO_EHDR);
+	printf("\tAT_SYSINFO_EHDR is 0x%lx\n", vdso_addr);
+	if (!vdso_addr || vdso_addr == -ENOENT) {
+		printf("[FAIL]\tgetauxval failed\n");
+		return 1;
+	}
+
+	/* to low for stack, to high for lib/data/code mappings */
+	dest_addr = 0x0a000000;
+	printf("[NOTE]\tMoving vDSO: [%lx, %lx] -> [%lx, %lx]\n",
+		vdso_addr, vdso_addr + VDSO_SIZE,
+		dest_addr, dest_addr + VDSO_SIZE);
+	new_addr = mremap((void *)vdso_addr, VDSO_SIZE, VDSO_SIZE,
+			MREMAP_FIXED|MREMAP_MAYMOVE, dest_addr);
+	if ((unsigned long)new_addr == (unsigned long)-1) {
+		printf("[FAIL]\tmremap failed (%d): %m\n", errno);
+		return 1;
+	}
+
+	asm volatile ("int $0x80" : : "a" (__NR_write), "b" (STDOUT_FILENO),
+			"c" (ok_string), "d" (strlen(ok_string)));
+	asm volatile ("int $0x80" : : "a" (__NR_exit), "b" (0));
+
+	return 0;
+}
+#endif
-- 
2.8.0

[toc] | [prev] | [next] | [standalone]

#1384547 — Re: [PATCHv5 3/3] selftest/x86: add mremap vdso 32-bit test

On Mon, Apr 18, 2016 at 6:43 AM, Dmitry Safonov <dsafonov@virtuozzo.com> wrote:
> Should print on success:
> [root@localhost ~]# ./test_mremap_vdso_32
>         AT_SYSINFO_EHDR is 0xf773f000
> [NOTE]  Moving vDSO: [f773f000, f7740000] -> [a000000, a001000]
> [OK]
> Or segfault if landing was bad (before patches):
> [root@localhost ~]# ./test_mremap_vdso_32
>         AT_SYSINFO_EHDR is 0xf774f000
> [NOTE]  Moving vDSO: [f774f000, f7750000] -> [a000000, a001000]
> Segmentation fault (core dumped)
>
> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> Cc: linux-kselftest@vger.kernel.org
> Suggested-by: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> ---
> v5: initial version
>
>  tools/testing/selftests/x86/Makefile           |  2 +-
>  tools/testing/selftests/x86/test_mremap_vdso.c | 72 ++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/x86/test_mremap_vdso.c
>
> diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
> index b47ebd170690..c7162b511ab0 100644
> --- a/tools/testing/selftests/x86/Makefile
> +++ b/tools/testing/selftests/x86/Makefile
> @@ -7,7 +7,7 @@ include ../lib.mk
>  TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_syscall \
>                         check_initial_reg_state sigreturn ldt_gdt iopl
>  TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \
> -                       test_FCMOV test_FCOMI test_FISTTP \
> +                       test_FCMOV test_FCOMI test_FISTTP test_mremap_vdso \
>                         vdso_restorer
>
>  TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
> diff --git a/tools/testing/selftests/x86/test_mremap_vdso.c b/tools/testing/selftests/x86/test_mremap_vdso.c
> new file mode 100644
> index 000000000000..a470790e2118
> --- /dev/null
> +++ b/tools/testing/selftests/x86/test_mremap_vdso.c
> @@ -0,0 +1,72 @@
> +/*
> + * 32-bit test to check vdso mremap.
> + *
> + * Copyright (c) 2016 Dmitry Safonov
> + * Suggested-by: Andrew Lutomirski
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +/*
> + * Can be built statically:
> + * gcc -Os -Wall -static -m32 test_mremap_vdso.c
> + */
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#include <sys/mman.h>
> +#include <sys/auxv.h>
> +#include <sys/syscall.h>
> +
> +#if !defined(__i386__)
> +int main(int argc, char **argv, char **envp)
> +{
> +       printf("[SKIP]\tNot a 32-bit x86 userspace\n");
> +       return 0;

What's wrong with testing on 64-bit systems?

> +}
> +#else
> +
> +#define PAGE_SIZE      4096
> +#define VDSO_SIZE      PAGE_SIZE

The vdso is frequently bigger than a page.

> +
> +int main(int argc, char **argv, char **envp)
> +{
> +       unsigned long vdso_addr, dest_addr;
> +       void *new_addr;
> +       const char *ok_string = "[OK]\n";
> +
> +       vdso_addr = getauxval(AT_SYSINFO_EHDR);
> +       printf("\tAT_SYSINFO_EHDR is 0x%lx\n", vdso_addr);
> +       if (!vdso_addr || vdso_addr == -ENOENT) {
> +               printf("[FAIL]\tgetauxval failed\n");
> +               return 1;

Let's make this [WARN] and return 0.  The vdso is optional, and
getauxval is missing on many systems.


> +       }
> +
> +       /* to low for stack, to high for lib/data/code mappings */
> +       dest_addr = 0x0a000000;

This could be make reliable -- map a big enough area PROT_NONE and use
that address.

> +       printf("[NOTE]\tMoving vDSO: [%lx, %lx] -> [%lx, %lx]\n",
> +               vdso_addr, vdso_addr + VDSO_SIZE,
> +               dest_addr, dest_addr + VDSO_SIZE);

fflush(stdout), please, for the benefit of test harnesses that use pipes.

--Andy

> +       new_addr = mremap((void *)vdso_addr, VDSO_SIZE, VDSO_SIZE,
> +                       MREMAP_FIXED|MREMAP_MAYMOVE, dest_addr);
> +       if ((unsigned long)new_addr == (unsigned long)-1) {
> +               printf("[FAIL]\tmremap failed (%d): %m\n", errno);
> +               return 1;
> +       }
> +
> +       asm volatile ("int $0x80" : : "a" (__NR_write), "b" (STDOUT_FILENO),
> +                       "c" (ok_string), "d" (strlen(ok_string)));
> +       asm volatile ("int $0x80" : : "a" (__NR_exit), "b" (0));
> +
> +       return 0;
> +}
> +#endif
> --
> 2.8.0
>



-- 
Andy Lutomirski
AMA Capital Management, LLC

[toc] | [prev] | [next] | [standalone]

#1382330 — [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()

Commit-ID:  abfb9498ee1327f534df92a7ecaea81a85913bae
Gitweb:     http://git.kernel.org/tip/abfb9498ee1327f534df92a7ecaea81a85913bae
Author:     Dmitry Safonov <dsafonov@virtuozzo.com>
AuthorDate: Mon, 18 Apr 2016 16:43:43 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 19 Apr 2016 10:44:52 +0200

x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()

The is_ia32_task()/is_x32_task() function names are a big misnomer: they
suggests that the compat-ness of a system call is a task property, which
is not true, the compatness of a system call purely depends on how it
was invoked through the system call layer.

A task may call 32-bit and 64-bit and x32 system calls without changing
any of its kernel visible state.

This specific minomer is also actively dangerous, as it might cause kernel
developers to use the wrong kind of security checks within system calls.

So rename it to in_{ia32,x32}_syscall().

Suggested-by: Andy Lutomirski <luto@amacapital.net>
Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
[ Expanded the changelog. ]
Acked-by: Andy Lutomirski <luto@kernel.org>
Cc: 0x7f454c46@gmail.com
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: akpm@linux-foundation.org
Cc: linux-mm@kvack.org
Link: http://lkml.kernel.org/r/1460987025-30360-1-git-send-email-dsafonov@virtuozzo.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/common.c            | 2 +-
 arch/x86/include/asm/compat.h      | 4 ++--
 arch/x86/include/asm/thread_info.h | 2 +-
 arch/x86/kernel/process_64.c       | 2 +-
 arch/x86/kernel/ptrace.c           | 2 +-
 arch/x86/kernel/signal.c           | 2 +-
 arch/x86/kernel/uprobes.c          | 2 +-
 7 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index e79d93d..ec138e5 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -191,7 +191,7 @@ long syscall_trace_enter_phase2(struct pt_regs *regs, u32 arch,
 
 long syscall_trace_enter(struct pt_regs *regs)
 {
-	u32 arch = is_ia32_task() ? AUDIT_ARCH_I386 : AUDIT_ARCH_X86_64;
+	u32 arch = in_ia32_syscall() ? AUDIT_ARCH_I386 : AUDIT_ARCH_X86_64;
 	unsigned long phase1_result = syscall_trace_enter_phase1(regs, arch);
 
 	if (phase1_result == 0)
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index ebb102e..5a3b2c1 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -307,7 +307,7 @@ static inline void __user *arch_compat_alloc_user_space(long len)
 	return (void __user *)round_down(sp - len, 16);
 }
 
-static inline bool is_x32_task(void)
+static inline bool in_x32_syscall(void)
 {
 #ifdef CONFIG_X86_X32_ABI
 	if (task_pt_regs(current)->orig_ax & __X32_SYSCALL_BIT)
@@ -318,7 +318,7 @@ static inline bool is_x32_task(void)
 
 static inline bool in_compat_syscall(void)
 {
-	return is_ia32_task() || is_x32_task();
+	return in_ia32_syscall() || in_x32_syscall();
 }
 #define in_compat_syscall in_compat_syscall	/* override the generic impl */
 
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index ffae84d..30c133a 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -255,7 +255,7 @@ static inline bool test_and_clear_restore_sigmask(void)
 	return true;
 }
 
-static inline bool is_ia32_task(void)
+static inline bool in_ia32_syscall(void)
 {
 #ifdef CONFIG_X86_32
 	return true;
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 50337ea..24d1b7f 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -210,7 +210,7 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
 	 */
 	if (clone_flags & CLONE_SETTLS) {
 #ifdef CONFIG_IA32_EMULATION
-		if (is_ia32_task())
+		if (in_ia32_syscall())
 			err = do_set_thread_area(p, -1,
 				(struct user_desc __user *)tls, 0);
 		else
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 32e9d9c..0f4d2a5 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1266,7 +1266,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
 			compat_ulong_t caddr, compat_ulong_t cdata)
 {
 #ifdef CONFIG_X86_X32_ABI
-	if (!is_ia32_task())
+	if (!in_ia32_syscall())
 		return x32_arch_ptrace(child, request, caddr, cdata);
 #endif
 #ifdef CONFIG_IA32_EMULATION
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 6408c09..2ebcc60 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -762,7 +762,7 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs)
 {
 #ifdef CONFIG_X86_64
-	if (is_ia32_task())
+	if (in_ia32_syscall())
 		return __NR_ia32_restart_syscall;
 #endif
 #ifdef CONFIG_X86_X32_ABI
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index bf4db6e..98b4dc8 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -516,7 +516,7 @@ struct uprobe_xol_ops {
 
 static inline int sizeof_long(void)
 {
-	return is_ia32_task() ? 4 : 8;
+	return in_ia32_syscall() ? 4 : 8;
 }
 
 static int default_pre_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs)

[toc] | [prev] | [next] | [standalone]

#1382399 — Re: [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()

* tip-bot for Dmitry Safonov <tipbot@zytor.com> wrote:

> Commit-ID:  abfb9498ee1327f534df92a7ecaea81a85913bae
> Gitweb:     http://git.kernel.org/tip/abfb9498ee1327f534df92a7ecaea81a85913bae
> Author:     Dmitry Safonov <dsafonov@virtuozzo.com>
> AuthorDate: Mon, 18 Apr 2016 16:43:43 +0300
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Tue, 19 Apr 2016 10:44:52 +0200
> 
> x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()

Btw., I'm not _entirely_ happy about the 'IA32' name, but went with this name for 
lack of a better alternative.

So we have 4 system call modes:

 - 64-bit native
 - 32-bit addresses with 64-bit arguments (x32)
 - 32-bit compat syscall (x86-32 compatibility on x86-64)
 - 32-bit native

and we have 2 bits of data that are per system call properties:

 - TS_COMPAT in thread_info->status is set/cleared dynamically by the compat 
   syscall entry code

 - a high bit in pt_regs->orig_ax tells us whether it's an x32 system call.

So I'd suggest the following renames to harmonize these concepts:

 - CONFIG_IA32_EMULATION   => CONFIG_X86_32_ABI
   this lines up nicely with: CONFIG_X86_X32_ABI

 - is_ia32_syscall() -> is_x86_32_syscall()
 - is_x32_syscall()  -> is_x86_x32_syscall()

 - is_compat_syscall() remains as-is.

... thoughts?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1382418 — Re: [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()

On Tue, Apr 19, 2016 at 01:15:30PM +0200, Ingo Molnar wrote:
> So I'd suggest the following renames to harmonize these concepts:
> 
>  - CONFIG_IA32_EMULATION   => CONFIG_X86_32_ABI
>    this lines up nicely with: CONFIG_X86_X32_ABI

Except that the only difference now is the "X" in the strings. So one
would need more coffee when staring at those. :)

> 
>  - is_ia32_syscall() -> is_x86_32_syscall()
>  - is_x32_syscall()  -> is_x86_x32_syscall()
> 
>  - is_compat_syscall() remains as-is.
> 
> ... thoughts?

Still, getting those names streamlined and logical is a step in the
right direction IMO.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply. Srsly.

[toc] | [prev] | [next] | [standalone]

#1382717 — Re: [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()

On April 19, 2016 4:35:01 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
>On Tue, Apr 19, 2016 at 01:15:30PM +0200, Ingo Molnar wrote:
>> So I'd suggest the following renames to harmonize these concepts:
>> 
>>  - CONFIG_IA32_EMULATION   => CONFIG_X86_32_ABI
>>    this lines up nicely with: CONFIG_X86_X32_ABI
>
>Except that the only difference now is the "X" in the strings. So one
>would need more coffee when staring at those. :)
>
>> 
>>  - is_ia32_syscall() -> is_x86_32_syscall()
>>  - is_x32_syscall()  -> is_x86_x32_syscall()
>> 
>>  - is_compat_syscall() remains as-is.
>> 
>> ... thoughts?
>
>Still, getting those names streamlined and logical is a step in the
>right direction IMO.

Let's use the i386 name instead of x86-32; that's what we use elsewhere in the kernel and is much easier to tell apart.  Also, less likely to be caught up in "is this i386 only or *any* 32-bit call, including x32"?
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

[toc] | [prev] | [next] | [standalone]

#1382666 — Re: [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()

On Tue, Apr 19, 2016 at 4:15 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * tip-bot for Dmitry Safonov <tipbot@zytor.com> wrote:
>
>> Commit-ID:  abfb9498ee1327f534df92a7ecaea81a85913bae
>> Gitweb:     http://git.kernel.org/tip/abfb9498ee1327f534df92a7ecaea81a85913bae
>> Author:     Dmitry Safonov <dsafonov@virtuozzo.com>
>> AuthorDate: Mon, 18 Apr 2016 16:43:43 +0300
>> Committer:  Ingo Molnar <mingo@kernel.org>
>> CommitDate: Tue, 19 Apr 2016 10:44:52 +0200
>>
>> x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()
>
> Btw., I'm not _entirely_ happy about the 'IA32' name, but went with this name for
> lack of a better alternative.
>
> So we have 4 system call modes:
>
>  - 64-bit native
>  - 32-bit addresses with 64-bit arguments (x32)
>  - 32-bit compat syscall (x86-32 compatibility on x86-64)
>  - 32-bit native
>
> and we have 2 bits of data that are per system call properties:
>
>  - TS_COMPAT in thread_info->status is set/cleared dynamically by the compat
>    syscall entry code
>
>  - a high bit in pt_regs->orig_ax tells us whether it's an x32 system call.
>
> So I'd suggest the following renames to harmonize these concepts:
>
>  - CONFIG_IA32_EMULATION   => CONFIG_X86_32_ABI
>    this lines up nicely with: CONFIG_X86_X32_ABI

I think I'd prefer a different interpretation: CONFIG_X86_32_ABI is
set if CONFIG_IA32_EMULATION is set *or* CONFIG_X86_32 is set.  There
is a lot of code that manually looks for that, because what it
actually cares about is "do we support 32-bit syscalls".  Also, with
the syscall cleanups I've been doing, a lot of the code is shared
between native 32-bit and 32-on-64 compat, so the distinction between
those two modes is slowly shrinking.

in_ia32_syscall() is consistent with that idea: it returns true on
native 32-bit kernels.

--Andy

[toc] | [prev] | [next] | [standalone]

#1384787 — Re: [tip:x86/asm] x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()

* Andy Lutomirski <luto@amacapital.net> wrote:

> On Tue, Apr 19, 2016 at 4:15 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * tip-bot for Dmitry Safonov <tipbot@zytor.com> wrote:
> >
> >> Commit-ID:  abfb9498ee1327f534df92a7ecaea81a85913bae
> >> Gitweb:     http://git.kernel.org/tip/abfb9498ee1327f534df92a7ecaea81a85913bae
> >> Author:     Dmitry Safonov <dsafonov@virtuozzo.com>
> >> AuthorDate: Mon, 18 Apr 2016 16:43:43 +0300
> >> Committer:  Ingo Molnar <mingo@kernel.org>
> >> CommitDate: Tue, 19 Apr 2016 10:44:52 +0200
> >>
> >> x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()
> >
> > Btw., I'm not _entirely_ happy about the 'IA32' name, but went with this name for
> > lack of a better alternative.
> >
> > So we have 4 system call modes:
> >
> >  - 64-bit native
> >  - 32-bit addresses with 64-bit arguments (x32)
> >  - 32-bit compat syscall (x86-32 compatibility on x86-64)
> >  - 32-bit native
> >
> > and we have 2 bits of data that are per system call properties:
> >
> >  - TS_COMPAT in thread_info->status is set/cleared dynamically by the compat
> >    syscall entry code
> >
> >  - a high bit in pt_regs->orig_ax tells us whether it's an x32 system call.
> >
> > So I'd suggest the following renames to harmonize these concepts:
> >
> >  - CONFIG_IA32_EMULATION   => CONFIG_X86_32_ABI
> >    this lines up nicely with: CONFIG_X86_X32_ABI
> 
> I think I'd prefer a different interpretation: CONFIG_X86_32_ABI is
> set if CONFIG_IA32_EMULATION is set *or* CONFIG_X86_32 is set.  There
> is a lot of code that manually looks for that, because what it
> actually cares about is "do we support 32-bit syscalls".  Also, with
> the syscall cleanups I've been doing, a lot of the code is shared
> between native 32-bit and 32-on-64 compat, so the distinction between
> those two modes is slowly shrinking.
> 
> in_ia32_syscall() is consistent with that idea: it returns true on
> native 32-bit kernels.

Ok, so how about:

 - rename CONFIG_IA32_EMULATION => CONFIG_X86_32_COMPAT

 - introduce CONFIG_X86_32_ABI to separate the 'convenience' functionality of 
   CONFIG_IA32_EMULATION from the ABI meaning.

 - rename is_ia32_syscall() -> is_x86_32_syscall()
 - rename is_x32_syscall()  -> is_x86_x32_syscall()

 - is_compat_syscall() remains as-is.

?

In the long run I'd like to get rid of two naming variants:

 - Fix all names that use 'IA32' that refer to 32-bit compat functionality,
   and only name the things 'IA32' that are truly Intel specific.

 - Fix all names that use 'emulation' when they really refer to 32-bit compat. We 
   don't actually emulate anything, we are just calling convention compatible with 
   very little overhead. The CPU is a fully 32-bit/64-bit dual mode hardware, 
   there's nothing that is emulated. Calling it IA32_EMULATION was a misnomer.

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web