linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND][PATCH v2] arch_prctl,x86 Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction
@ 2016-09-14 21:08 Kyle Huey
  2016-09-14 21:08 ` [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32 Kyle Huey
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Kyle Huey @ 2016-09-14 21:08 UTC (permalink / raw)
  To: Robert O'Callahan; +Cc: linux-kernel, Borislav Petkov, Andy Lutomirski

(Resending because I screwed up the cover email, sorry about that.)

rr (http://rr-project.org/), a userspace record-and-replay reverse-
execution debugger, would like to trap and emulate the CPUID instruction.
This would allow us to a) mask away certain hardware features that rr does
not support (e.g. RDRAND) and b) enable trace portability across machines
by providing constant results.

4 patches follow, the first 3 to the kernel, and the final patch to man-pages.

The following changes have been	 made since v1:

Suggested by Borislav Petkov:
  - Uses arch_prctl instead of prctl.
    - Uses rdmsr_safe.
      - Added sample man-pages patch.
        - Various functions are renamed, style fixes.

Suggested by Andy Lutomirski:
  - Added a cpufeature bit to show up in /proc/cpuinfo.
    - Added sane behavior  in Xen, by masking away the MSR_PLATFORM_INFO bit
        showing support for this feature for now.
	  - Added a selftest, clarifying the bit is preserved on fork/exec.

The following issues were raised and are not addressed:

Use of cpuid within interrupt handlers: as Linus pointed out, CPUID only
faults at cpl>0, so this is not a concern.

Use a static_key instead of a TIF: I don't believe this solves anything.
There are currently 8 free TIF bits (after this patch), and it's always
possible to move this (or others) later if they are needed. Even if we were
to use a static_key we would still need to maintain state about which tasks
are subject to CPUID faulting and which are not somewhere else.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32.
  2016-09-14 21:08 [RESEND][PATCH v2] arch_prctl,x86 Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction Kyle Huey
@ 2016-09-14 21:08 ` Kyle Huey
  2016-09-14 21:59   ` Dmitry Safonov
  2016-09-14 21:08 ` [RESEND][PATCH v2 2/3] x86 Test and expose CPUID faulting capabilities in /proc/cpuinfo Kyle Huey
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Kyle Huey @ 2016-09-14 21:08 UTC (permalink / raw)
  To: Robert O'Callahan
  Cc: linux-kernel, Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Al Viro, Milosz Tanski, Dmitry V. Levin, David Howells,
	Zach Brown, Eric B Munson, Peter Zijlstra, Jiri Slaby,
	Michael S. Tsirkin, Andrey Ryabinin, Paul Gortmaker,
	Borislav Petkov, Dmitry Vyukov, Dmitry Safonov, Mateusz Guzik

Signed-off-by: Kyle Huey <khuey@kylehuey.com>
---
 arch/x86/entry/syscalls/syscall_32.tbl |  1 +
 arch/x86/kernel/process.c              | 80 ++++++++++++++++++++++++++++++++++
 arch/x86/kernel/process_64.c           | 66 ----------------------------
 3 files changed, 81 insertions(+), 66 deletions(-)

diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index f848572..3b6965b 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -386,3 +386,4 @@
 377	i386	copy_file_range		sys_copy_file_range
 378	i386	preadv2			sys_preadv2			compat_sys_preadv2
 379	i386	pwritev2		sys_pwritev2			compat_sys_pwritev2
+380	i386	arch_prctl		sys_arch_prctl
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 62c0b0e..0f857c3 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -20,6 +20,7 @@
 #include <linux/cpuidle.h>
 #include <trace/events/power.h>
 #include <linux/hw_breakpoint.h>
+#include <linux/syscalls.h>
 #include <asm/cpu.h>
 #include <asm/apic.h>
 #include <asm/syscalls.h>
@@ -32,6 +33,7 @@
 #include <asm/tlbflush.h>
 #include <asm/mce.h>
 #include <asm/vm86.h>
+#include <asm/prctl.h>
 
 /*
  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
@@ -567,3 +569,81 @@ unsigned long get_wchan(struct task_struct *p)
 	} while (count++ < 16 && p->state != TASK_RUNNING);
 	return 0;
 }
+
+long do_arch_prctl(struct task_struct *task, int code, unsigned long arg2)
+{
+	int ret = 0;
+	int doit = task == current;
+	int is_32 = IS_ENABLED(CONFIG_IA32_EMULATION) && test_thread_flag(TIF_IA32);
+	int cpu;
+
+	switch (code) {
+#ifdef CONFIG_X86_64
+	case ARCH_SET_GS:
+		if (is_32)
+			return -EINVAL;
+		if (arg2 >= TASK_SIZE_MAX)
+			return -EPERM;
+		cpu = get_cpu();
+		task->thread.gsindex = 0;
+		task->thread.gsbase = arg2;
+		if (doit) {
+			load_gs_index(0);
+			ret = wrmsrl_safe(MSR_KERNEL_GS_BASE, arg2);
+		}
+		put_cpu();
+		break;
+	case ARCH_SET_FS:
+		if (is_32)
+			return -EINVAL;
+		/* Not strictly needed for fs, but do it for symmetry
+		   with gs */
+		if (arg2 >= TASK_SIZE_MAX)
+			return -EPERM;
+		cpu = get_cpu();
+		task->thread.fsindex = 0;
+		task->thread.fsbase = arg2;
+		if (doit) {
+			/* set the selector to 0 to not confuse __switch_to */
+			loadsegment(fs, 0);
+			ret = wrmsrl_safe(MSR_FS_BASE, arg2);
+		}
+		put_cpu();
+		break;
+	case ARCH_GET_FS: {
+		unsigned long base;
+
+		if (is_32)
+			return -EINVAL;
+		if (doit)
+			rdmsrl(MSR_FS_BASE, base);
+		else
+			base = task->thread.fsbase;
+		ret = put_user(base, (unsigned long __user *)arg2);
+		break;
+	}
+	case ARCH_GET_GS: {
+		unsigned long base;
+
+		if (is_32)
+			return -EINVAL;
+		if (doit)
+			rdmsrl(MSR_KERNEL_GS_BASE, base);
+		else
+			base = task->thread.gsbase;
+		ret = put_user(base, (unsigned long __user *)arg2);
+		break;
+	}
+#endif
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+SYSCALL_DEFINE2(arch_prctl, int, code, unsigned long, arg2)
+{
+	return do_arch_prctl(current, code, arg2);
+}
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 63236d8..e8c6302 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -524,72 +524,6 @@ void set_personality_ia32(bool x32)
 }
 EXPORT_SYMBOL_GPL(set_personality_ia32);
 
-long do_arch_prctl(struct task_struct *task, int code, unsigned long addr)
-{
-	int ret = 0;
-	int doit = task == current;
-	int cpu;
-
-	switch (code) {
-	case ARCH_SET_GS:
-		if (addr >= TASK_SIZE_MAX)
-			return -EPERM;
-		cpu = get_cpu();
-		task->thread.gsindex = 0;
-		task->thread.gsbase = addr;
-		if (doit) {
-			load_gs_index(0);
-			ret = wrmsrl_safe(MSR_KERNEL_GS_BASE, addr);
-		}
-		put_cpu();
-		break;
-	case ARCH_SET_FS:
-		/* Not strictly needed for fs, but do it for symmetry
-		   with gs */
-		if (addr >= TASK_SIZE_MAX)
-			return -EPERM;
-		cpu = get_cpu();
-		task->thread.fsindex = 0;
-		task->thread.fsbase = addr;
-		if (doit) {
-			/* set the selector to 0 to not confuse __switch_to */
-			loadsegment(fs, 0);
-			ret = wrmsrl_safe(MSR_FS_BASE, addr);
-		}
-		put_cpu();
-		break;
-	case ARCH_GET_FS: {
-		unsigned long base;
-		if (doit)
-			rdmsrl(MSR_FS_BASE, base);
-		else
-			base = task->thread.fsbase;
-		ret = put_user(base, (unsigned long __user *)addr);
-		break;
-	}
-	case ARCH_GET_GS: {
-		unsigned long base;
-		if (doit)
-			rdmsrl(MSR_KERNEL_GS_BASE, base);
-		else
-			base = task->thread.gsbase;
-		ret = put_user(base, (unsigned long __user *)addr);
-		break;
-	}
-
-	default:
-		ret = -EINVAL;
-		break;
-	}
-
-	return ret;
-}
-
-long sys_arch_prctl(int code, unsigned long addr)
-{
-	return do_arch_prctl(current, code, addr);
-}
-
 unsigned long KSTK_ESP(struct task_struct *task)
 {
 	return task_pt_regs(task)->sp;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RESEND][PATCH v2 2/3] x86 Test and expose CPUID faulting capabilities in /proc/cpuinfo
  2016-09-14 21:08 [RESEND][PATCH v2] arch_prctl,x86 Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction Kyle Huey
  2016-09-14 21:08 ` [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32 Kyle Huey
@ 2016-09-14 21:08 ` Kyle Huey
  2016-09-14 21:08 ` [RESEND][PATCH v2 3/3] x86,arch_prctl Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction Kyle Huey
  2016-09-14 21:10 ` [PATCH (man-pages)] arch_prctl.2: Note new support on x86-32, ARCH_[GET|SET]_CPUID Kyle Huey
  3 siblings, 0 replies; 9+ messages in thread
From: Kyle Huey @ 2016-09-14 21:08 UTC (permalink / raw)
  To: Robert O'Callahan
  Cc: linux-kernel, Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Boris Ostrovsky, David Vrabel, Juergen Gross, Peter Zijlstra,
	Huang Rui, Dave Hansen, Rafael J. Wysocki, Len Brown,
	Srinivas Pandruvada, Aravind Gopalakrishnan, Alexander Shishkin,
	Vladimir Zapolskiy, Kristen Carlson Accardi,
	moderated list:XEN HYPERVISOR INTERFACE

Xen advertises the underlying support for CPUID faulting but not does pass
through writes to the relevant MSR, nor does it virtualize it, so it does
not actually work. For now mask off the relevant bit on MSR_PLATFORM_INFO.

Signed-off-by: Kyle Huey <khuey@kylehuey.com>
---
 arch/x86/include/asm/cpufeatures.h |  1 +
 arch/x86/include/asm/msr-index.h   |  1 +
 arch/x86/kernel/cpu/scattered.c    | 14 ++++++++++++++
 arch/x86/xen/enlighten.c           |  3 +++
 4 files changed, 19 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 92a8308..78b9d06 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -190,6 +190,7 @@
 
 #define X86_FEATURE_CPB		( 7*32+ 2) /* AMD Core Performance Boost */
 #define X86_FEATURE_EPB		( 7*32+ 3) /* IA32_ENERGY_PERF_BIAS support */
+#define X86_FEATURE_CPUID_FAULT ( 7*32+ 4) /* Intel CPUID faulting */
 
 #define X86_FEATURE_HW_PSTATE	( 7*32+ 8) /* AMD HW-PState */
 #define X86_FEATURE_PROC_FEEDBACK ( 7*32+ 9) /* AMD ProcFeedbackInterface */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 56f4c66..83908d5 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -41,6 +41,7 @@
 #define MSR_IA32_PERFCTR1		0x000000c2
 #define MSR_FSB_FREQ			0x000000cd
 #define MSR_PLATFORM_INFO		0x000000ce
+#define CPUID_FAULTING_SUPPORT		(1UL << 31)
 
 #define MSR_NHM_SNB_PKG_CST_CFG_CTL	0x000000e2
 #define NHM_C3_AUTO_DEMOTE		(1UL << 25)
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 8cb57df..d502da1 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -24,6 +24,17 @@ enum cpuid_regs {
 	CR_EBX
 };
 
+static int supports_cpuid_faulting(void)
+{
+	unsigned int lo, hi;
+
+	if (rdmsr_safe(MSR_PLATFORM_INFO, &lo, &hi) == 0 &&
+	    (lo & CPUID_FAULTING_SUPPORT))
+		return 1;
+	else
+		return 0;
+}
+
 void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
 {
 	u32 max_level;
@@ -54,4 +65,7 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
 		if (regs[cb->reg] & (1 << cb->bit))
 			set_cpu_cap(c, cb->feature);
 	}
+
+	if (supports_cpuid_faulting())
+		set_cpu_cap(c, X86_FEATURE_CPUID_FAULT);
 }
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index b86ebb1..2c47f0c 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1050,6 +1050,9 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err)
 #endif
 			val &= ~X2APIC_ENABLE;
 		break;
+	case MSR_PLATFORM_INFO:
+		val &= ~CPUID_FAULTING_SUPPORT;
+		break;
 	}
 	return val;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RESEND][PATCH v2 3/3] x86,arch_prctl Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction
  2016-09-14 21:08 [RESEND][PATCH v2] arch_prctl,x86 Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction Kyle Huey
  2016-09-14 21:08 ` [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32 Kyle Huey
  2016-09-14 21:08 ` [RESEND][PATCH v2 2/3] x86 Test and expose CPUID faulting capabilities in /proc/cpuinfo Kyle Huey
@ 2016-09-14 21:08 ` Kyle Huey
  2016-09-14 21:10 ` [PATCH (man-pages)] arch_prctl.2: Note new support on x86-32, ARCH_[GET|SET]_CPUID Kyle Huey
  3 siblings, 0 replies; 9+ messages in thread
From: Kyle Huey @ 2016-09-14 21:08 UTC (permalink / raw)
  To: Robert O'Callahan
  Cc: linux-kernel, Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Shuah Khan, Peter Zijlstra (Intel),
	Rafael J. Wysocki, Len Brown, Srinivas Pandruvada,
	Aravind Gopalakrishnan, Alexander Shishkin, Vladimir Zapolskiy,
	Oleg Nesterov, Kees Cook, Dmitry Safonov, Andrey Ryabinin,
	Jiri Slaby, Michael S. Tsirkin, Paul Gortmaker, Denys Vlasenko,
	Dave Hansen, open list:KERNEL SELFTEST FRAMEWORK

Intel supports faulting on the CPUID instruction in newer processors. Bit
31 of MSR_PLATFORM_INFO advertises support for this feature. It is
documented in detail in Section 2.3.2 of
http://www.intel.com/content/dam/www/public/us/en/documents/application-notes/virtualization-technology-flexmigration-application-note.pdf

Signed-off-by: Kyle Huey <khuey@kylehuey.com>
---
 arch/x86/include/asm/msr-index.h          |   1 +
 arch/x86/include/asm/thread_info.h        |   4 +-
 arch/x86/include/uapi/asm/prctl.h         |   6 +
 arch/x86/kernel/process.c                 |  81 +++++++++++
 tools/testing/selftests/x86/Makefile      |   2 +-
 tools/testing/selftests/x86/cpuid-fault.c | 223 ++++++++++++++++++++++++++++++
 6 files changed, 315 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/x86/cpuid-fault.c

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 83908d5..4aebec2 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -53,6 +53,7 @@
 #define MSR_MTRRcap			0x000000fe
 #define MSR_IA32_BBL_CR_CTL		0x00000119
 #define MSR_IA32_BBL_CR_CTL3		0x0000011e
+#define MSR_MISC_FEATURES_ENABLES	0x00000140
 
 #define MSR_IA32_SYSENTER_CS		0x00000174
 #define MSR_IA32_SYSENTER_ESP		0x00000175
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 8b7c8d8..ec93976 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -93,6 +93,7 @@ struct thread_info {
 #define TIF_SECCOMP		8	/* secure computing */
 #define TIF_USER_RETURN_NOTIFY	11	/* notify kernel of userspace return */
 #define TIF_UPROBE		12	/* breakpointed or singlestepping */
+#define TIF_NOCPUID		15	/* CPUID is not accessible in userland */
 #define TIF_NOTSC		16	/* TSC is not accessible in userland */
 #define TIF_IA32		17	/* IA32 compatibility process */
 #define TIF_FORK		18	/* ret_from_fork */
@@ -117,6 +118,7 @@ struct thread_info {
 #define _TIF_SECCOMP		(1 << TIF_SECCOMP)
 #define _TIF_USER_RETURN_NOTIFY	(1 << TIF_USER_RETURN_NOTIFY)
 #define _TIF_UPROBE		(1 << TIF_UPROBE)
+#define _TIF_NOCPUID		(1 << TIF_NOCPUID)
 #define _TIF_NOTSC		(1 << TIF_NOTSC)
 #define _TIF_IA32		(1 << TIF_IA32)
 #define _TIF_FORK		(1 << TIF_FORK)
@@ -146,7 +148,7 @@ struct thread_info {
 
 /* flags to check in __switch_to() */
 #define _TIF_WORK_CTXSW							\
-	(_TIF_IO_BITMAP|_TIF_NOTSC|_TIF_BLOCKSTEP)
+	(_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP)
 
 #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)
 #define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h
index 3ac5032..c087e55 100644
--- a/arch/x86/include/uapi/asm/prctl.h
+++ b/arch/x86/include/uapi/asm/prctl.h
@@ -6,4 +6,10 @@
 #define ARCH_GET_FS 0x1003
 #define ARCH_GET_GS 0x1004
 
+/* Get/set the process' ability to use the CPUID instruction */
+#define ARCH_GET_CPUID 0x1005
+#define ARCH_SET_CPUID 0x1006
+# define ARCH_CPUID_ENABLE		1	/* allow the use of the CPUID instruction */
+# define ARCH_CPUID_SIGSEGV		2	/* throw a SIGSEGV instead of reading the CPUID */
+
 #endif /* _ASM_X86_PRCTL_H */
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 0f857c3..5fc8e9d 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -193,6 +193,69 @@ int set_tsc_mode(unsigned int val)
 	return 0;
 }
 
+static void switch_cpuid_faulting(bool on)
+{
+	if (on)
+		msr_set_bit(MSR_MISC_FEATURES_ENABLES, 0);
+	else
+		msr_clear_bit(MSR_MISC_FEATURES_ENABLES, 0);
+}
+
+static void disable_cpuid(void)
+{
+	preempt_disable();
+	if (!test_and_set_thread_flag(TIF_NOCPUID))
+		/*
+		 * Must flip the CPU state synchronously with
+		 * TIF_NOCPUID in the current running context.
+		 */
+		switch_cpuid_faulting(true);
+	preempt_enable();
+}
+
+static void enable_cpuid(void)
+{
+	preempt_disable();
+	if (test_and_clear_thread_flag(TIF_NOCPUID))
+		/*
+		 * Must flip the CPU state synchronously with
+		 * TIF_NOCPUID in the current running context.
+		 */
+		switch_cpuid_faulting(false);
+	preempt_enable();
+}
+
+int get_cpuid_mode(unsigned long adr)
+{
+	unsigned int val;
+
+	if (test_thread_flag(TIF_NOCPUID))
+		val = ARCH_CPUID_SIGSEGV;
+	else
+		val = ARCH_CPUID_ENABLE;
+
+	return put_user(val, (unsigned int __user *)adr);
+}
+
+int set_cpuid_mode(struct task_struct *task, unsigned long val)
+{
+	/* Only disable/enable_cpuid() if it is supported on this hardware. */
+	bool cpuid_fault_supported = static_cpu_has(X86_FEATURE_CPUID_FAULT);
+
+	if (val == ARCH_CPUID_ENABLE && cpuid_fault_supported) {
+		if (task_no_new_privs(task) && test_thread_flag(TIF_NOCPUID))
+			return -EACCES;
+
+		enable_cpuid();
+	} else if (val == ARCH_CPUID_SIGSEGV && cpuid_fault_supported) {
+		disable_cpuid();
+	} else {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 		      struct tss_struct *tss)
 {
@@ -212,6 +275,15 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 		update_debugctlmsr(debugctl);
 	}
 
+	if (test_tsk_thread_flag(prev_p, TIF_NOCPUID) ^
+	    test_tsk_thread_flag(next_p, TIF_NOCPUID)) {
+		/* prev and next are different */
+		if (test_tsk_thread_flag(next_p, TIF_NOCPUID))
+			switch_cpuid_faulting(true);
+		else
+			switch_cpuid_faulting(false);
+	}
+
 	if (test_tsk_thread_flag(prev_p, TIF_NOTSC) ^
 	    test_tsk_thread_flag(next_p, TIF_NOTSC)) {
 		/* prev and next are different */
@@ -635,6 +707,15 @@ long do_arch_prctl(struct task_struct *task, int code, unsigned long arg2)
 		break;
 	}
 #endif
+	case ARCH_GET_CPUID: {
+		ret = get_cpuid_mode(arg2);
+		break;
+	}
+	case ARCH_SET_CPUID: {
+		ret = set_cpuid_mode(task, arg2);
+		break;
+	}
+
 	default:
 		ret = -EINVAL;
 		break;
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index 4f747ee..fbf34d3 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -5,7 +5,7 @@ include ../lib.mk
 .PHONY: all all_32 all_64 warn_32bit_failure clean
 
 TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_syscall test_mremap_vdso \
-			check_initial_reg_state sigreturn ldt_gdt iopl mpx-mini-test
+			check_initial_reg_state sigreturn ldt_gdt iopl mpx-mini-test cpuid-fault
 TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \
 			test_FCMOV test_FCOMI test_FISTTP \
 			vdso_restorer
diff --git a/tools/testing/selftests/x86/cpuid-fault.c b/tools/testing/selftests/x86/cpuid-fault.c
new file mode 100644
index 0000000..d72b723
--- /dev/null
+++ b/tools/testing/selftests/x86/cpuid-fault.c
@@ -0,0 +1,223 @@
+
+/*
+ * Tests for arch_prctl(ARCH_GET_CPUID, ...) / prctl(ARCH_SET_CPUID, ...)
+ *
+ * Basic test to test behaviour of ARCH_GET_CPUID and ARCH_SET_CPUID
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <signal.h>
+#include <inttypes.h>
+#include <cpuid.h>
+#include <errno.h>
+#include <sys/wait.h>
+
+#include <sys/prctl.h>
+#include <linux/prctl.h>
+
+const char *cpuid_names[] = {
+	[0] = "[not set]",
+	[ARCH_CPUID_ENABLE] = "ARCH_CPUID_ENABLE",
+	[ARCH_CPUID_SIGSEGV] = "ARCH_CPUID_SIGSEGV",
+};
+
+int arch_prctl(int code, unsigned long arg2)
+{
+	return syscall(SYS_arch_prctl, code, arg2);
+}
+
+int cpuid(unsigned int *eax, unsigned int *ebx, unsigned int *ecx,
+	  unsigned int *edx)
+{
+	return __get_cpuid(0, eax, ebx, ecx, edx);
+}
+
+int do_child_exec_test(int eax, int ebx, int ecx, int edx)
+{i
+	int cpuid_val = 0, child = 0, status = 0;
+
+	printf("arch_prctl(ARCH_GET_CPUID, &cpuid_val); ");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_GET_CPUID, (unsigned long)&cpuid_val) != 0)
+		exit(42);
+
+	printf("cpuid_val == %s\n", cpuid_names[cpuid_val]);
+	if (cpuid_val != ARCH_CPUID_SIGSEGV)
+		exit(42);
+
+	if ((child = fork()) == 0)
+		execl("/proc/self/exe", "cpuid-fault", NULL);
+
+	/* That will blow up almost immediately, since dl/libc use cpuid. */
+	if (child != waitpid(child, &status, 0))
+		exit(42);
+
+	if (!WIFSIGNALED(status) || WTERMSIG(status) != SIGSEGV)
+		exit(42);
+
+	return 0;
+}
+
+int child_received_signal;
+
+void child_sigsegv_cb(int sig)
+{
+	int cpuid_val = 0;
+
+	child_received_signal = 1;
+	printf("[ SIG_SEGV ]\n");
+	printf("arch_prctl(ARCH_GET_CPUID, &cpuid_val); ");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_GET_CPUID, (unsigned long)&cpuid_val) != 0)
+		exit(42);
+
+	printf("cpuid_val == %s\n", cpuid_names[cpuid_val]);
+	printf("arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_ENABLE)\n");
+	fflush(stdout);
+	if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_ENABLE) != 0)
+		exit(errno);
+
+	printf("cpuid() == ");
+}
+
+int do_child_test(void)
+{
+	unsigned int eax = 0, ebx = 0, ecx = 0, edx = 0;
+
+	signal(SIGSEGV, child_sigsegv_cb);
+
+	/* the child starts out with cpuid disabled, the signal handler
+	 * attempts to enable and retry
+	 */
+	printf("cpuid() == ");
+	fflush(stdout);
+	cpuid(&eax, &ebx, &ecx, &edx);
+	printf("{%x, %x, %x, %x}\n", eax, ebx, ecx, edx);
+	return child_received_signal ? 0 : 42;
+}
+
+void sigsegv_cb(int sig)
+{
+	int cpuid_val = 0;
+
+	printf("[ SIG_SEGV ]\n");
+	printf("arch_prctl(ARCH_GET_CPUID, &cpuid_val); ");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_GET_CPUID, (unsigned long)&cpuid_val) != 0)
+		exit(42);
+
+	printf("cpuid_val == %s\n", cpuid_names[cpuid_val]);
+	printf("arch_prctl(ARC_SET_CPUID, ARCH_CPUID_ENABLE)\n");
+	fflush(stdout);
+	if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_ENABLE) != 0)
+		exit(42);
+
+	printf("cpuid() == ");
+}
+
+int main(void)
+{
+	int cpuid_val = 0, child = 0, status = 0;
+	unsigned int eax = 0, ebx = 0, ecx = 0, edx = 0;
+
+	signal(SIGSEGV, sigsegv_cb);
+
+	cpuid(&eax, &ebx, &ecx, &edx);
+	printf("cpuid() == {%x, %x, %x, %x}\n", eax, ebx, ecx, edx);
+	printf("arch_prctl(ARCH_GET_CPUID, &cpuid_val); ");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_GET_CPUID, (unsigned long)&cpuid_val) != 0) {
+		if (errno == EINVAL) {
+			printf("ARCH_GET_CPUID is unsupported on this system.");
+			fflush(stdout);
+			exit(0); /* no ARCH_GET_CPUID on this system */
+		} else {
+			exit(42);
+		}
+	}
+
+	printf("cpuid_val == %s\n", cpuid_names[cpuid_val]);
+	cpuid(&eax, &ebx, &ecx, &edx);
+	printf("cpuid() == {%x, %x, %x, %x}\n", eax, ebx, ecx, edx);
+	printf("arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_ENABLE)\n");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_ENABLE) != 0) {
+		if (errno == EINVAL) {
+			printf("ARCH_SET_CPUID is unsupported on this system.");
+			fflush(stdout);
+			exit(0); /* no ARCH_SET_CPUID on this system */
+		} else {
+			exit(42);
+		}
+	}
+
+
+	cpuid(&eax, &ebx, &ecx, &edx);
+	printf("cpuid() == {%x, %x, %x, %x}\n", eax, ebx, ecx, edx);
+	printf("arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_SIGSEGV)\n");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_SIGSEGV) == -1)
+		exit(42);
+
+	printf("cpuid() == ");
+	fflush(stdout);
+	eax = ebx = ecx = edx = 0;
+	cpuid(&eax, &ebx, &ecx, &edx);
+	printf("{%x, %x, %x, %x}\n", eax, ebx, ecx, edx);
+	printf("arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_SIGSEGV)\n");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_SIGSEGV) == -1)
+		exit(42);
+
+	printf("do_child_test\n");
+	fflush(stdout);
+	if ((child = fork()) == 0)
+		return do_child_test();
+
+	if (child != waitpid(child, &status, 0))
+		exit(42);
+
+	if (WEXITSTATUS(status) != 0)
+		exit(42);
+
+	printf("prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)\n");
+	fflush(stdout);
+	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) != 0)
+		exit(42);
+
+	printf("do_child_test\n");
+	fflush(stdout);
+	if ((child = fork()) == 0)
+		return do_child_test();
+
+	if (child != waitpid(child, &status, 0))
+		exit(42);
+
+	if (WEXITSTATUS(status) != EACCES)
+		exit(42);
+
+	printf("do_child_exec_test\n");
+	fflush(stdout);
+	if ((child = fork()) == 0)
+		return do_child_exec_test(eax, ebx, ecx, edx);
+
+	if (child != waitpid(child, &status, 0))
+		exit(42);
+
+	if (WEXITSTATUS(status) != 0)
+		exit(42);
+
+	printf("All tests passed!\n");
+	fflush(stdout);
+	exit(EXIT_SUCCESS);
+}
+
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH (man-pages)] arch_prctl.2: Note new support on x86-32, ARCH_[GET|SET]_CPUID.
  2016-09-14 21:08 [RESEND][PATCH v2] arch_prctl,x86 Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction Kyle Huey
                   ` (2 preceding siblings ...)
  2016-09-14 21:08 ` [RESEND][PATCH v2 3/3] x86,arch_prctl Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction Kyle Huey
@ 2016-09-14 21:10 ` Kyle Huey
  3 siblings, 0 replies; 9+ messages in thread
From: Kyle Huey @ 2016-09-14 21:10 UTC (permalink / raw)
  To: Robert O'Callahan, mtk.manpages
  Cc: linux-kernel, Borislav Petkov, Andy Lutomirski, linux-man

Signed-off-by: Kyle Huey <khuey@kylehuey.com>
---
 man2/arch_prctl.2 | 73 +++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 60 insertions(+), 13 deletions(-)

diff --git a/man2/arch_prctl.2 b/man2/arch_prctl.2
index 989d369..c388797 100644
--- a/man2/arch_prctl.2
+++ b/man2/arch_prctl.2
@@ -22,7 +22,7 @@
 .\" the source, must acknowledge the copyright and authors of this work.
 .\" %%%LICENSE_END
 .\"
-.TH ARCH_PRCTL 2 2015-02-21 "Linux" "Linux Programmer's Manual"
+.TH ARCH_PRCTL 2 2016-09-14 "Linux" "Linux Programmer's Manual"
 .SH NAME
 arch_prctl \- set architecture-specific thread state
 .SH SYNOPSIS
@@ -31,8 +31,8 @@ arch_prctl \- set architecture-specific thread state
 .br
 .B #include <sys/prctl.h>
 .sp
-.BI "int arch_prctl(int " code ", unsigned long " addr );
-.BI "int arch_prctl(int " code ", unsigned long *" addr );
+.BI "int arch_prctl(int " code ", unsigned long " arg2 );
+.BI "int arch_prctl(int " code ", unsigned long *" arg2 );
 .fi
 .SH DESCRIPTION
 The
@@ -41,22 +41,47 @@ function sets architecture-specific process or thread state.
 .I code
 selects a subfunction
 and passes argument
-.I addr
+.I arg2
 to it;
-.I addr
+.I arg2
 is interpreted as either an
 .I "unsigned long"
 for the "set" operations, or as an
 .IR "unsigned long\ *" ,
 for the "get" operations.
 .LP
+Subfunctions for both x86-64 and x86-32 are:
+.TP
+.B ARCH_GET_CPUID " (since Linux 4.X)"
+Return the state of the flag determining whether the
+.I cpuid
+instruction can be executed by the process, in the
+.I unsigned long
+pointed to by
+.IR arg2 .
+.TP
+.B ARCH_SET_CPUID " (since Linux 4.X)"
+Set the state of the flag determining whether the
+.I cpuid
+instruction can be executed by the process. Pass
+.B ARCH_CPUID_ENABLE
+in
+.I arg2
+to allow it to be executed, or
+.B ARCH_CPUID_SIGSEGV
+to generate a
+.B SIGSEGV
+when the process tries to execute the
+.I cpuid
+instruction. This flag is propagated across fork and exec.
+.LP
 Subfunctions for x86-64 are:
 .TP
 .B ARCH_SET_FS
 Set the 64-bit base for the
 .I FS
 register to
-.IR addr .
+.IR arg2 .
 .TP
 .B ARCH_GET_FS
 Return the 64-bit base value for the
@@ -64,13 +89,13 @@ Return the 64-bit base value for the
 register of the current thread in the
 .I unsigned long
 pointed to by
-.IR addr .
+.IR arg2 .
 .TP
 .B ARCH_SET_GS
 Set the 64-bit base for the
 .I GS
 register to
-.IR addr .
+.IR arg2 .
 .TP
 .B ARCH_GET_GS
 Return the 64-bit base value for the
@@ -78,7 +103,7 @@ Return the 64-bit base value for the
 register of the current thread in the
 .I unsigned long
 pointed to by
-.IR addr .
+.IR arg2 .
 .SH RETURN VALUE
 On success,
 .BR arch_prctl ()
@@ -87,26 +112,48 @@ returns 0; on error, \-1 is returned, and
 is set to indicate the error.
 .SH ERRORS
 .TP
+.B EACCES
+.I code
+is
+.B ARCH_SET_CPUID
+and
+.I arg2
+is
+.B ARCH_CPUID_ENABLE
+and cpuid was previously disabled with
+.B ARCH_CPUID_SIGSEGV
+and the
+.I no_new_privs
+bit is set on this thread.
+.TP
 .B EFAULT
-.I addr
+.I arg2
 points to an unmapped address or is outside the process address space.
 .TP
 .B EINVAL
 .I code
 is not a valid subcommand.
 .TP
+.B EINVAL
+.I code
+is
+.B ARCH_SET_CPUID
+and
+.I cpuid
+faulting is not supported on this machine.
+.TP
 .B EPERM
-.I addr
+.I arg2
 is outside the process address space.
 .\" .SH AUTHOR
 .\" Man page written by Andi Kleen.
 .SH CONFORMING TO
 .BR arch_prctl ()
-is a Linux/x86-64 extension and should not be used in programs intended
+is a Linux/x86 extension and should not be used in programs intended
 to be portable.
 .SH NOTES
 .BR arch_prctl ()
-is supported only on Linux/x86-64 for 64-bit programs currently.
+is supported only on Linux/x86 currently.
 
 The 64-bit base changes when a new 32-bit segment selector is loaded.
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32.
  2016-09-14 21:08 ` [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32 Kyle Huey
@ 2016-09-14 21:59   ` Dmitry Safonov
  2016-09-14 22:08     ` Kyle Huey
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Safonov @ 2016-09-14 21:59 UTC (permalink / raw)
  To: Kyle Huey
  Cc: Robert O'Callahan, linux-kernel, Borislav Petkov,
	Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Al Viro, Milosz Tanski, Dmitry V. Levin, David Howells,
	Zach Brown, Eric B Munson, Peter Zijlstra, Jiri Slaby,
	Michael S. Tsirkin, Andrey Ryabinin, Paul Gortmaker,
	Borislav Petkov, Dmitry Vyukov, Dmitry Safonov, Mateusz Guzik

2016-09-15 0:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
> ---
>  arch/x86/entry/syscalls/syscall_32.tbl |  1 +
>  arch/x86/kernel/process.c              | 80 ++++++++++++++++++++++++++++++++++
>  arch/x86/kernel/process_64.c           | 66 ----------------------------
>  3 files changed, 81 insertions(+), 66 deletions(-)
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index f848572..3b6965b 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -386,3 +386,4 @@
>  377    i386    copy_file_range         sys_copy_file_range
>  378    i386    preadv2                 sys_preadv2                     compat_sys_preadv2
>  379    i386    pwritev2                sys_pwritev2                    compat_sys_pwritev2
> +380    i386    arch_prctl              sys_arch_prctl

Why not define it as other 32-bit syscalls with compat_sys_ prefix
with the help of COMPAT_SYSCALL_DEFINE() macro?
Then you could omit code moving, drop is_32 helper.
I miss something obvious?

-- 
             Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32.
  2016-09-14 21:59   ` Dmitry Safonov
@ 2016-09-14 22:08     ` Kyle Huey
  2016-09-14 22:29       ` Dmitry Safonov
  0 siblings, 1 reply; 9+ messages in thread
From: Kyle Huey @ 2016-09-14 22:08 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: Robert O'Callahan, open list, Borislav Petkov,
	Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Al Viro, Milosz Tanski, Dmitry V. Levin, David Howells,
	Zach Brown, Eric B Munson, Peter Zijlstra, Jiri Slaby,
	Michael S. Tsirkin, Andrey Ryabinin, Paul Gortmaker,
	Borislav Petkov, Dmitry Vyukov, Dmitry Safonov, Mateusz Guzik,
	Dave Hansen

On Wed, Sep 14, 2016 at 2:59 PM, Dmitry Safonov <0x7f454c46@gmail.com> wrote:
> 2016-09-15 0:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
>> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
>> ---
>>  arch/x86/entry/syscalls/syscall_32.tbl |  1 +
>>  arch/x86/kernel/process.c              | 80 ++++++++++++++++++++++++++++++++++
>>  arch/x86/kernel/process_64.c           | 66 ----------------------------
>>  3 files changed, 81 insertions(+), 66 deletions(-)
>>
>> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
>> index f848572..3b6965b 100644
>> --- a/arch/x86/entry/syscalls/syscall_32.tbl
>> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
>> @@ -386,3 +386,4 @@
>>  377    i386    copy_file_range         sys_copy_file_range
>>  378    i386    preadv2                 sys_preadv2                     compat_sys_preadv2
>>  379    i386    pwritev2                sys_pwritev2                    compat_sys_pwritev2
>> +380    i386    arch_prctl              sys_arch_prctl
>
> Why not define it as other 32-bit syscalls with compat_sys_ prefix
> with the help of COMPAT_SYSCALL_DEFINE() macro?
> Then you could omit code moving, drop is_32 helper.
> I miss something obvious?

The code will have to move regardless, because right now do_arch_prctl
is in process-64.c which is only compiled on a 64 bit kernel.

As I told Dave Hansen in the non-RESEND thread (not sure why
git-send-email didn't put him in this one ...) I considered doing a
compat_sys_arch_prctl that would reject the relevant arch_prctls that
don't apply on 32 bit but I didn't see any prior art for it (in my
admittedly non-exhaustive search).

- Kyle

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32.
  2016-09-14 22:08     ` Kyle Huey
@ 2016-09-14 22:29       ` Dmitry Safonov
  2016-09-15  1:01         ` Kyle Huey
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Safonov @ 2016-09-14 22:29 UTC (permalink / raw)
  To: Kyle Huey
  Cc: Robert O'Callahan, open list, Borislav Petkov,
	Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Al Viro, Milosz Tanski, Dmitry V. Levin, David Howells,
	Zach Brown, Eric B Munson, Peter Zijlstra, Jiri Slaby,
	Michael S. Tsirkin, Andrey Ryabinin, Paul Gortmaker,
	Borislav Petkov, Dmitry Vyukov, Dmitry Safonov, Mateusz Guzik,
	Dave Hansen

2016-09-15 1:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
> On Wed, Sep 14, 2016 at 2:59 PM, Dmitry Safonov <0x7f454c46@gmail.com> wrote:
>> 2016-09-15 0:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
>>> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
>>> ---
>>>  arch/x86/entry/syscalls/syscall_32.tbl |  1 +
>>>  arch/x86/kernel/process.c              | 80 ++++++++++++++++++++++++++++++++++
>>>  arch/x86/kernel/process_64.c           | 66 ----------------------------
>>>  3 files changed, 81 insertions(+), 66 deletions(-)
>>>
>>> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
>>> index f848572..3b6965b 100644
>>> --- a/arch/x86/entry/syscalls/syscall_32.tbl
>>> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
>>> @@ -386,3 +386,4 @@
>>>  377    i386    copy_file_range         sys_copy_file_range
>>>  378    i386    preadv2                 sys_preadv2                     compat_sys_preadv2
>>>  379    i386    pwritev2                sys_pwritev2                    compat_sys_pwritev2
>>> +380    i386    arch_prctl              sys_arch_prctl
>>
>> Why not define it as other 32-bit syscalls with compat_sys_ prefix
>> with the help of COMPAT_SYSCALL_DEFINE() macro?
>> Then you could omit code moving, drop is_32 helper.
>> I miss something obvious?
>
> The code will have to move regardless, because right now do_arch_prctl
> is in process-64.c which is only compiled on a 64 bit kernel.

Why? This code will not work anyway for 32-bit in your patches
by obscuring it with is_32.

> As I told Dave Hansen in the non-RESEND thread (not sure why
> git-send-email didn't put him in this one ...) I considered doing a
> compat_sys_arch_prctl that would reject the relevant arch_prctls that
> don't apply on 32 bit but I didn't see any prior art for it (in my
> admittedly non-exhaustive search).

Well, you could just add to 64-bit do_arch_prctl() new cases for your
prctls - that would be just a two-lines for each new prctl.
Also add compat_sys_ and define *only* what's needed there for you,
do not add there ARCH_{SET,GET}_{FS,GS}.
Does this make sense?

-- 
             Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32.
  2016-09-14 22:29       ` Dmitry Safonov
@ 2016-09-15  1:01         ` Kyle Huey
  0 siblings, 0 replies; 9+ messages in thread
From: Kyle Huey @ 2016-09-15  1:01 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: Robert O'Callahan, open list, Borislav Petkov,
	Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Al Viro, Milosz Tanski, Dmitry V. Levin, David Howells,
	Zach Brown, Eric B Munson, Peter Zijlstra, Jiri Slaby,
	Michael S. Tsirkin, Andrey Ryabinin, Paul Gortmaker,
	Borislav Petkov, Dmitry Vyukov, Dmitry Safonov, Mateusz Guzik,
	Dave Hansen

On Wed, Sep 14, 2016 at 3:29 PM, Dmitry Safonov <0x7f454c46@gmail.com> wrote:
> 2016-09-15 1:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
>> On Wed, Sep 14, 2016 at 2:59 PM, Dmitry Safonov <0x7f454c46@gmail.com> wrote:
>>> 2016-09-15 0:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
>>>> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
>>>> ---
>>>>  arch/x86/entry/syscalls/syscall_32.tbl |  1 +
>>>>  arch/x86/kernel/process.c              | 80 ++++++++++++++++++++++++++++++++++
>>>>  arch/x86/kernel/process_64.c           | 66 ----------------------------
>>>>  3 files changed, 81 insertions(+), 66 deletions(-)
>>>>
>>>> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
>>>> index f848572..3b6965b 100644
>>>> --- a/arch/x86/entry/syscalls/syscall_32.tbl
>>>> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
>>>> @@ -386,3 +386,4 @@
>>>>  377    i386    copy_file_range         sys_copy_file_range
>>>>  378    i386    preadv2                 sys_preadv2                     compat_sys_preadv2
>>>>  379    i386    pwritev2                sys_pwritev2                    compat_sys_pwritev2
>>>> +380    i386    arch_prctl              sys_arch_prctl
>>>
>>> Why not define it as other 32-bit syscalls with compat_sys_ prefix
>>> with the help of COMPAT_SYSCALL_DEFINE() macro?
>>> Then you could omit code moving, drop is_32 helper.
>>> I miss something obvious?
>>
>> The code will have to move regardless, because right now do_arch_prctl
>> is in process-64.c which is only compiled on a 64 bit kernel.
>
> Why? This code will not work anyway for 32-bit in your patches
> by obscuring it with is_32.
>
>> As I told Dave Hansen in the non-RESEND thread (not sure why
>> git-send-email didn't put him in this one ...) I considered doing a
>> compat_sys_arch_prctl that would reject the relevant arch_prctls that
>> don't apply on 32 bit but I didn't see any prior art for it (in my
>> admittedly non-exhaustive search).
>
> Well, you could just add to 64-bit do_arch_prctl() new cases for your
> prctls - that would be just a two-lines for each new prctl.
> Also add compat_sys_ and define *only* what's needed there for you,
> do not add there ARCH_{SET,GET}_{FS,GS}.
> Does this make sense?

Yeah, I should have spoken more clearly.  We'll need some
implementation of the syscall outside of process_64.c.  But we could
leave the 64 bit specific stuff behind in it.   Dave Hansen suggested
something similar (though without the compat_sys_bit)

>FWIW, I don't think it would be horrible to leave the existing
> do_arch_prctl() code in process_64.h and call it
> do_64_bit_only_something_arch_prctl(), and only call in to it from the
> generic do_arch_prctl().  You really have one reason for all the "if
> (is_32)"'s and it would be nice to document why in one single place.

- Kyle

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-09-15  1:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-14 21:08 [RESEND][PATCH v2] arch_prctl,x86 Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction Kyle Huey
2016-09-14 21:08 ` [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32 Kyle Huey
2016-09-14 21:59   ` Dmitry Safonov
2016-09-14 22:08     ` Kyle Huey
2016-09-14 22:29       ` Dmitry Safonov
2016-09-15  1:01         ` Kyle Huey
2016-09-14 21:08 ` [RESEND][PATCH v2 2/3] x86 Test and expose CPUID faulting capabilities in /proc/cpuinfo Kyle Huey
2016-09-14 21:08 ` [RESEND][PATCH v2 3/3] x86,arch_prctl Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction Kyle Huey
2016-09-14 21:10 ` [PATCH (man-pages)] arch_prctl.2: Note new support on x86-32, ARCH_[GET|SET]_CPUID Kyle Huey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).