Skip to contentSkip to conversation

compat: Introduce and use in_compat_syscall

PATCH In review Andy Lutomirski 6 participants 28 messages Jan 25, 2016 – Mar 23, 2016

Subsystems (22)
Networking DRM drivers x86 Staging +18 more subsystems
lore
Share & tools
lore
#0 lore raw

v2 cover compat: Introduce and use in_compat_syscall

Linux distinguishes compat syscalls from normal syscalls in two ways. First, compat syscalls use separate entry points. This is entirely sensible. Second, code paths that are common to compat and non-compat syscalls can query is_compat_task() to check whether they are being called from a compat syscall or not.

The latter is problematic. Two architectures explicitly allow tasks to issue syscalls for which the task bitness and the syscall bitness don't match. On x86, is_compat_task returns true if the current syscall is a compat syscall, which is a different condition from being a compat task. This oddity is confusing. On SPARC, is_compat_task does what it sounds like, which means that user programs can cause is_compat_task to fail to match the syscall entry used. That could expose bugs.

This series introduces in_compat_syscall() as a new way to query the syscall type. On SPARC, it really does check the syscall bitness. Later patches change all of the non arch-specific is_compat_task callers to use in_compat_syscall. The last patch removes is_compat_task on x86, which will prevent this confusion from recurring.

I've cc'd the maintainers of all archs that support compat. If your arch makes it possible for a malicious user process to invoke a compat syscall in a nominally 64-bit task or vice versa and your arch is not x86 or SPARC, please tell me.

Davem, can you check whether I handled SPARC correctly?

Andy Lutomirski (16):
compat: Add in_compat_syscall to ask whether we're in a compat syscall sparc/compat: Provide an accurate in_compat_syscall implementation sparc/syscall: Fix syscall_get_arch
seccomp: Check in_compat_syscall, not is_compat_task, in strict mode
ptrace: in PEEK_SIGINFO, check syscall bitness, not task bitness
auditsc: For seccomp events, log syscall compat state using
in_compat_syscall
staging/lustre: Switch from is_compat_task to in_compat_syscall
ext4: In ext4_dir_llseek, check syscall bitness directly net/sctp: Use in_compat_syscall for sctp_getsockopt_connectx3 net/xfrm_user: Use in_compat_syscall to deny compat syscalls
firewire: Use in_compat_syscall to check ioctl compatness
efivars: Use in_compat_syscall to check for compat callers
amdkfd: Use in_compat_syscall to check open() caller type
input: Redefine INPUT_COMPAT_TEST as in_compat_syscall()
uhid: Check write() bitness using in_compat_syscall x86/compat: Remove is_compat_task

 arch/sparc/include/asm/compat.h                      |  6 ++++++
 arch/sparc/include/asm/syscall.h                     |  9 ++++++++-
 arch/x86/include/asm/compat.h                        |  3 ++-
 arch/x86/include/asm/ftrace.h                        |  2 +-
 arch/x86/kernel/process_64.c                         |  2 +-
 drivers/firewire/core-cdev.c                         |  4 ++--
 drivers/firmware/efi/efivars.c                       |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c             |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_process.c             |  2 +-
 drivers/hid/uhid.c                                   |  2 +-
 drivers/input/input-compat.h                         | 12 +-----------
 drivers/staging/lustre/lustre/llite/llite_internal.h |  2 +-
 fs/ext4/dir.c                                        |  2 +-
 include/linux/compat.h                               | 15 +++++++++++++++
 kernel/auditsc.c                                     |  4 ++--
 kernel/ptrace.c                                      |  2 +-
 kernel/seccomp.c                                     |  4 ++--
 net/sctp/socket.c                                    |  2 +-
 net/xfrm/xfrm_user.c                                 |  2 +-
 19 files changed, 49 insertions(+), 30 deletions(-)
Signature · 1 line
2.5.0
#1 lore raw

PATCH v2 1/16 compat: Add in_compat_syscall to ask whether we're in a compat syscall

A lot of code currently abuses is_compat_task to determine this.

---
 include/linux/compat.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
include/linux/compat.h +15 -0
index a76c9172b2eb..f911bcec618f 100644
@@ -5,6 +5,8 @@
  * syscall compatibility layer.
  */
 
+#include <linux/types.h>
+
 #ifdef CONFIG_COMPAT
 
 #include <linux/stat.h>
@@ -713,9 +715,22 @@ asmlinkage long compat_sys_sched_rr_get_interval(compat_pid_t pid,
 
 asmlinkage long compat_sys_fanotify_mark(int, unsigned int, __u32, __u32,
 					    int, const char __user *);
+
+/*
+ * For most but not all architectures, "am I in a compat syscall?" and
+ * "am I a compat task?" are the same question.  For architectures on which
+ * they aren't the same question, arch code can override in_compat_syscall.
+ */
+
+#ifndef in_compat_syscall
+static inline bool in_compat_syscall(void) { return is_compat_task(); }
+#endif
+
 #else
 
 #define is_compat_task() (0)
+static inline bool in_compat_syscall(void) { return false; }
 
 #endif /* CONFIG_COMPAT */
+
 #endif /* _LINUX_COMPAT_H */
Signature · 1 line
2.5.0
#2 lore raw

PATCH v2 2/16 sparc/compat: Provide an accurate in_compat_syscall implementation

In reply to Andy Lutomirski

On sparc64 compat-enabled kernels, any task can make 32-bit and 64-bit syscalls. is_compat_task returns true in 32-bit tasks, which does not necessarily imply that the current syscall is 32-bit.

Provide an in_compat_syscall implementation that checks whether the current syscall is compat.

As far as I know, sparc is the only architecture on which is_compat_task checks the compat status of the task and on which the compat status of a syscall can differ from the compat status of the task. On x86, is_compat_task checks the syscall type, not the task type.

---
 arch/sparc/include/asm/compat.h | 6 ++++++
 1 file changed, 6 insertions(+)
arch/sparc/include/asm/compat.h +6 -0
index 830502fe62b4..5467404857fc 100644
@@ -307,4 +307,10 @@ static inline int is_compat_task(void)
 	return test_thread_flag(TIF_32BIT);
 }
 
+static inline bool in_compat_syscall(void)
+{
+	return pt_regs_trap_type(current_pt_regs()) == 0x110;
+}
+#define in_compat_syscall in_compat_syscall
+
 #endif /* _ASM_SPARC64_COMPAT_H */
Signature · 1 line
2.5.0
maintainer Acked-by on v2 2/16
#3 lore raw

Re: [PATCH v2 02/16] sparc/compat: Provide an accurate in_compat_syscall implementation

From: Andy Lutomirski <[email protected]>
Date: Mon, 25 Jan 2016 14:24:16 -0800

Quoted 7 lines

On sparc64 compat-enabled kernels, any task can make 32-bit and 64-bit syscalls. is_compat_task returns true in 32-bit tasks, which does not necessarily imply that the current syscall is 32-bit.

Provide an in_compat_syscall implementation that checks whether the current syscall is compat.

Quoted 7 lines

As far as I know, sparc is the only architecture on which is_compat_task checks the compat status of the task and on which the compat status of a syscall can differ from the compat status of the task. On x86, is_compat_task checks the syscall type, not the task type.

#4 lore raw

Re: [PATCH v2 02/16] sparc/compat: Provide an accurate in_compat_syscall implementation

In reply to Andy Lutomirski
Quoted 23 lines · Andy Lutomirski

On sparc64 compat-enabled kernels, any task can make 32-bit and 64-bit syscalls. is_compat_task returns true in 32-bit tasks, which does not necessarily imply that the current syscall is 32-bit.

Provide an in_compat_syscall implementation that checks whether the current syscall is compat.

As far as I know, sparc is the only architecture on which is_compat_task checks the compat status of the task and on which the compat status of a syscall can differ from the compat status of the task. On x86, is_compat_task checks the syscall type, not the task type.

---
 arch/sparc/include/asm/compat.h | 6 ++++++
 1 file changed, 6 insertions(+)
arch/sparc/include/asm/compat.h +0 -0
index 830502fe62b4..5467404857fc 100644

arch/sparc/include/asm/compat.h static inline int is_compat_task(void) · line 312

Quoted 6 lines
arch/sparc/include/asm/compat.h +3 -0
@@ -307,4 +307,10 @@ static inline int is_compat_task(void)
 	return test_thread_flag(TIF_32BIT);
 }
 
+static inline bool in_compat_syscall(void)
+{
+	return pt_regs_trap_type(current_pt_regs()) == 0x110;

Could you please add a comment about where 0x110 comes from. I at least failed to track this down.

Sam

maintainer
#5 lore raw

Re: [PATCH v2 02/16] sparc/compat: Provide an accurate in_compat_syscall implementation

From: Sam Ravnborg <[email protected]>
Date: Tue, 26 Jan 2016 07:29:51 +0100

Quoted 2 lines

Could you please add a comment about where 0x110 comes from. I at least failed to track this down.

Frankly I'm fine with this. Someone who understands sparc64 can look at the trap table around entry 0x110 and see:

tl0_resv10e: BTRAP(0x10e) BTRAP(0x10f)
tl0_linux32: LINUX_32BIT_SYSCALL_TRAP
tl0_oldlinux64: LINUX_64BIT_SYSCALL_TRAP

#6 lore raw

Re: [PATCH v2 02/16] sparc/compat: Provide an accurate in_compat_syscall implementation

Quoted 6 lines · David Miller

From: Sam Ravnborg <[email protected]>
Date: Tue, 26 Jan 2016 07:29:51 +0100

Quoted 2 lines

Could you please add a comment about where 0x110 comes from. I at least failed to track this down.

Quoted 6 lines

Frankly I'm fine with this. Someone who understands sparc64 can look at the trap table around entry 0x110 and see:

tl0_resv10e: BTRAP(0x10e) BTRAP(0x10f)
tl0_linux32: LINUX_32BIT_SYSCALL_TRAP
tl0_oldlinux64: LINUX_64BIT_SYSCALL_TRAP

If one realise to look in the trap table in the first place - yes.

Adding a short:

/* Check if this is LINUX_32BIT_SYSCALL_TRAP */
Would make wonders to readability.

Sam

#7 lore raw

Re: [PATCH v2 02/16] sparc/compat: Provide an accurate in_compat_syscall implementation

Quoted 14 lines · Sam Ravnborg
Quoted 6 lines · David Miller

From: Sam Ravnborg <[email protected]>
Date: Tue, 26 Jan 2016 07:29:51 +0100

Quoted 2 lines

Could you please add a comment about where 0x110 comes from. I at least failed to track this down.

Quoted 6 lines

Frankly I'm fine with this. Someone who understands sparc64 can look at the trap table around entry 0x110 and see:

tl0_resv10e:  BTRAP(0x10e) BTRAP(0x10f)
tl0_linux32:  LINUX_32BIT_SYSCALL_TRAP
tl0_oldlinux64:       LINUX_64BIT_SYSCALL_TRAP
Quoted 6 lines

If one realise to look in the trap table in the first place - yes.

Adding a short:

/* Check if this is LINUX_32BIT_SYSCALL_TRAP */
Would make wonders to readability.

I'll add a comment in v2.

--Andy

Signature · 2 lines
Andy Lutomirski
AMA Capital Management, LLC
maintainer
#8 lore raw

Re: [PATCH v2 02/16] sparc/compat: Provide an accurate in_compat_syscall implementation

In reply to Sam Ravnborg

From: Sam Ravnborg <[email protected]>
Date: Tue, 26 Jan 2016 18:44:41 +0100

Quoted 14 lines · David Miller
Quoted 6 lines

From: Sam Ravnborg <[email protected]>
Date: Tue, 26 Jan 2016 07:29:51 +0100

Quoted 2 lines

Could you please add a comment about where 0x110 comes from. I at least failed to track this down.

Quoted 6 lines

Frankly I'm fine with this. Someone who understands sparc64 can look at the trap table around entry 0x110 and see:

tl0_resv10e: BTRAP(0x10e) BTRAP(0x10f)
tl0_linux32: LINUX_32BIT_SYSCALL_TRAP
tl0_oldlinux64: LINUX_64BIT_SYSCALL_TRAP

Quoted 6 lines

If one realise to look in the trap table in the first place - yes.

Adding a short:

/* Check if this is LINUX_32BIT_SYSCALL_TRAP */
Would make wonders to readability.

Fair enough.

#9 lore raw

PATCH v2 3/16 sparc/syscall: Fix syscall_get_arch

In reply to Andy Lutomirski

Sparc's syscall_get_arch was buggy: it returned the task arch, not the syscall arch. This could confuse seccomp and audit.

I don't think this is as bad for seccomp as it looks: sparc's 32-bit and 64-bit syscalls are numbered the same.

---
 arch/sparc/include/asm/syscall.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
arch/sparc/include/asm/syscall.h +8 -1
index 49f71fd5b56e..1757cd6c521b 100644
@@ -3,6 +3,7 @@
 
 #include <uapi/linux/audit.h>
 #include <linux/kernel.h>
+#include <linux/compat.h>
 #include <linux/sched.h>
 #include <asm/ptrace.h>
 #include <asm/thread_info.h>
@@ -128,7 +129,13 @@ static inline void syscall_set_arguments(struct task_struct *task,
 
 static inline int syscall_get_arch(void)
 {
-	return is_32bit_task() ? AUDIT_ARCH_SPARC : AUDIT_ARCH_SPARC64;
+#if defined(CONFIG_SPARC64) && defined(CONFIG_COMPAT)
+	return in_compat_syscall() ? AUDIT_ARCH_SPARC : AUDIT_ARCH_SPARC64;
+#elif defined(CONFIG_SPARC64)
+	return AUDIT_ARCH_SPARC64;
+#else
+	return AUDIT_ARCH_SPARC;
+#endif
 }
 
 #endif /* __ASM_SPARC_SYSCALL_H */
Signature · 1 line
2.5.0
#10 lore raw

PATCH v2 4/16 seccomp: Check in_compat_syscall, not is_compat_task, in strict mode

In reply to Andy Lutomirski

Seccomp wants to know the syscall bitness, not the caller task bitness, when it selects the syscall whitelist.

As far as I know, this makes no difference on any architecture, so it's not a security problem. (It generates identical code everywhere except sparc, and, on sparc, the syscall numbering is the same for both ABIs.)

---
 kernel/seccomp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/seccomp.c +2 -2
index 580ac2d4024f..26858fa43a60 100644
@@ -395,7 +395,7 @@ seccomp_prepare_user_filter(const char __user *user_filter)
 	struct seccomp_filter *filter = ERR_PTR(-EFAULT);
 
 #ifdef CONFIG_COMPAT
-	if (is_compat_task()) {
+	if (in_compat_syscall()) {
 		struct compat_sock_fprog fprog32;
 		if (copy_from_user(&fprog32, user_filter, sizeof(fprog32)))
 			goto out;
@@ -529,7 +529,7 @@ static void __secure_computing_strict(int this_syscall)
 {
 	int *syscall_whitelist = mode1_syscalls;
 #ifdef CONFIG_COMPAT
-	if (is_compat_task())
+	if (in_compat_syscall())
 		syscall_whitelist = mode1_syscalls_32;
 #endif
 	do {
Signature · 1 line
2.5.0
#11 lore raw

PATCH v2 5/16 ptrace: in PEEK_SIGINFO, check syscall bitness, not task bitness

In reply to Andy Lutomirski

Users of the 32-bit ptrace() ABI expect the full 32-bit ABI. siginfo translation should check ptrace() ABI, not caller task ABI.

This is an ABI change on SPARC. Let's hope that no one relied on the old buggy ABI.

---
 kernel/ptrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/ptrace.c +1 -1
index 2341efe7fe02..c79b91d09e35 100644
@@ -681,7 +681,7 @@ static int ptrace_peek_siginfo(struct task_struct *child,
 			break;
 
 #ifdef CONFIG_COMPAT
-		if (unlikely(is_compat_task())) {
+		if (unlikely(in_compat_syscall())) {
 			compat_siginfo_t __user *uinfo = compat_ptr(data);
 
 			if (copy_siginfo_to_user32(uinfo, &info) ||
Signature · 1 line
2.5.0
#12 lore raw

PATCH v2 6/16 auditsc: For seccomp events, log syscall compat state using in_compat_syscall

In reply to Andy Lutomirski

Except on SPARC, this is what the code always did. SPARC compat seccomp was buggy, although the impact of the bug was limited because SPARC 32-bit and 64-bit syscall numbers are the same.

---
 kernel/auditsc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/auditsc.c +2 -2
index 195ffaee50b9..7d0e3cf8abe1 100644
@@ -2412,8 +2412,8 @@ void __audit_seccomp(unsigned long syscall, long signr, int code)
 		return;
 	audit_log_task(ab);
 	audit_log_format(ab, " sig=%ld arch=%x syscall=%ld compat=%d ip=0x%lx code=0x%x",
-			 signr, syscall_get_arch(), syscall, is_compat_task(),
-			 KSTK_EIP(current), code);
+			 signr, syscall_get_arch(), syscall,
+			 in_compat_syscall(), KSTK_EIP(current), code);
 	audit_log_end(ab);
 }
 
Signature · 1 line
2.5.0
#13 lore raw

PATCH v2 7/16 staging/lustre: Switch from is_compat_task to in_compat_syscall

In reply to Andy Lutomirski

AFAICT, lustre is trying to determine syscall bitness. Use the new accessor.

---
 drivers/staging/lustre/lustre/llite/llite_internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/lustre/lustre/llite/llite_internal.h +1 -1
index 845e992ca5fc..373470410e05 100644
@@ -647,7 +647,7 @@ static inline int ll_need_32bit_api(struct ll_sb_info *sbi)
 #if BITS_PER_LONG == 32
 	return 1;
 #elif defined(CONFIG_COMPAT)
-	return unlikely(is_compat_task() || (sbi->ll_flags & LL_SBI_32BIT_API));
+	return unlikely(in_compat_syscall() || (sbi->ll_flags & LL_SBI_32BIT_API));
 #else
 	return unlikely(sbi->ll_flags & LL_SBI_32BIT_API);
 #endif
Signature · 1 line
2.5.0
#14 lore raw

PATCH v2 8/16 ext4: In ext4_dir_llseek, check syscall bitness directly

In reply to Andy Lutomirski

ext4 treats directory offsets differently for 32-bit and 64-bit callers. Check the caller type using in_compat_syscall, not is_compat_task. This changes behavior on SPARC slightly.

---
 fs/ext4/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ext4/dir.c +1 -1
index 1d1bca74f844..6395456edea6 100644
@@ -276,7 +276,7 @@ errout:
 static inline int is_32bit_api(void)
 {
 #ifdef CONFIG_COMPAT
-	return is_compat_task();
+	return in_compat_syscall();
 #else
 	return (BITS_PER_LONG == 32);
 #endif
Signature · 1 line
2.5.0
#15 lore raw

PATCH v2 9/16 net/sctp: Use in_compat_syscall for sctp_getsockopt_connectx3

In reply to Andy Lutomirski

SCTP unfortunately has a different ABI for SCTP_SOCKOPT_CONNECTX3 for 32-bit and 64-bit callers. Use in_compat_syscall to correctly distinguish them on all architectures.

---
 net/sctp/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/sctp/socket.c +1 -1
index 9bb80ec4c08f..20ab6b2bbbb9 100644
@@ -1389,7 +1389,7 @@ static int sctp_getsockopt_connectx3(struct sock *sk, int len,
 	int err = 0;
 
 #ifdef CONFIG_COMPAT
-	if (is_compat_task()) {
+	if (in_compat_syscall()) {
 		struct compat_sctp_getaddrs_old param32;
 
 		if (len < sizeof(param32))
Signature · 1 line
2.5.0
#16 lore raw

PATCH v2 10/16 net/xfrm_user: Use in_compat_syscall to deny compat syscalls

In reply to Andy Lutomirski

The code wants to prevent compat code from receiving messages. Use in_compat_syscall for this.

---
 net/xfrm/xfrm_user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/xfrm/xfrm_user.c +1 -1
index 805681a7d356..2cc7af858c6f 100644
@@ -2449,7 +2449,7 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	int type, err;
 
 #ifdef CONFIG_COMPAT
-	if (is_compat_task())
+	if (in_compat_syscall())
 		return -ENOTSUPP;
 #endif
 
Signature · 1 line
2.5.0
#17 lore raw

PATCH v2 11/16 firewire: Use in_compat_syscall to check ioctl compatness

In reply to Andy Lutomirski

Firewire was using is_compat_task to check whether it was in a compat ioctl or a non-compat ioctl. Use is_compat_syscall instead so it works properly on all architectures.

---
 drivers/firewire/core-cdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/firewire/core-cdev.c +2 -2
index 36a7c2d89a01..aee149bdf4c0 100644
@@ -221,7 +221,7 @@ struct inbound_phy_packet_event {
 #ifdef CONFIG_COMPAT
 static void __user *u64_to_uptr(u64 value)
 {
-	if (is_compat_task())
+	if (in_compat_syscall())
 		return compat_ptr(value);
 	else
 		return (void __user *)(unsigned long)value;
@@ -229,7 +229,7 @@ static void __user *u64_to_uptr(u64 value)
 
 static u64 uptr_to_u64(void __user *ptr)
 {
-	if (is_compat_task())
+	if (in_compat_syscall())
 		return ptr_to_compat(ptr);
 	else
 		return (u64)(unsigned long)ptr;
Signature · 1 line
2.5.0
#18 lore raw

PATCH v2 12/16 efivars: Use in_compat_syscall to check for compat callers

In reply to Andy Lutomirski

This should make no difference on any architecture, as x86's historical is_compat_task behavior really did check whether the calling syscall was a compat syscall. x86's is_compat_task is going away, though.

---
 drivers/firmware/efi/efivars.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/firmware/efi/efivars.c +1 -1
index 756eca8c4cf8..21c3b2016b2d 100644
@@ -231,7 +231,7 @@ sanity_check(struct efi_variable *var, efi_char16_t *name, efi_guid_t vendor,
 
 static inline bool is_compat(void)
 {
-	if (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())
+	if (IS_ENABLED(CONFIG_COMPAT) && in_compat_syscall())
 		return true;
 
 	return false;
Signature · 1 line
2.5.0
#19 lore raw

PATCH v2 13/16 amdkfd: Use in_compat_syscall to check open() caller type

In reply to Andy Lutomirski

amdkfd wants to know syscall type, not task type. Check directly.

Unfortunately, amdkfd is making nasty assumptions that a process' bitness is a well-defined constant thing. This isn't the case on x86. I don't know how much this matters, but this patch has no effect on generated code on x86, so amdkfd is equally broken with and without this patch.

---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c +1 -1
index d2b49c026cf6..07ac724e3ec9 100644
@@ -107,7 +107,7 @@ static int kfd_open(struct inode *inode, struct file *filep)
 	if (iminor(inode) != 0)
 		return -ENODEV;
 
-	is_32bit_user_mode = is_compat_task();
+	is_32bit_user_mode = in_compat_syscall();
 
 	if (is_32bit_user_mode == true) {
 		dev_warn(kfd_device,
drivers/gpu/drm/amd/amdkfd/kfd_process.c +1 -1
index 9be007081b72..fd1a90a0f435 100644
@@ -311,7 +311,7 @@ static struct kfd_process *create_process(const struct task_struct *thread)
 		goto err_process_pqm_init;
 
 	/* init process apertures*/
-	process->is_32bit_user_mode = is_compat_task();
+	process->is_32bit_user_mode = in_compat_syscall();
 	if (kfd_init_apertures(process) != 0)
 		goto err_init_apretures;
 
Signature · 1 line
2.5.0
#20 lore raw

PATCH v2 14/16 input: Redefine INPUT_COMPAT_TEST as in_compat_syscall()

In reply to Andy Lutomirski

The input compat code should work like all other compat code: for 32-bit syscalls, use the 32-bit ABI and for 64-bit syscalls, use the 64-bit ABI. We have a helper for that (in_compat_syscall()): just use it.

---
 drivers/input/input-compat.h | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)
drivers/input/input-compat.h +1 -11
index 148f66fe3205..0f25878d5fa2 100644
@@ -17,17 +17,7 @@
 
 #ifdef CONFIG_COMPAT
 
-/* Note to the author of this code: did it ever occur to
-   you why the ifdefs are needed? Think about it again. -AK */
-#if defined(CONFIG_X86_64) || defined(CONFIG_TILE)
-#  define INPUT_COMPAT_TEST is_compat_task()
-#elif defined(CONFIG_S390)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_31BIT)
-#elif defined(CONFIG_MIPS)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT_ADDR)
-#else
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT)
-#endif
+#define INPUT_COMPAT_TEST in_compat_syscall()
 
 struct input_event_compat {
 	struct compat_timeval time;
Signature · 1 line
2.5.0
maintainer
#21 lore raw

Re: [PATCH v2 14/16] input: Redefine INPUT_COMPAT_TEST as in_compat_syscall()

Hi Andy,

Quoted 21 lines · Andy Lutomirski

The input compat code should work like all other compat code: for 32-bit syscalls, use the 32-bit ABI and for 64-bit syscalls, use the 64-bit ABI. We have a helper for that (in_compat_syscall()): just use it.

---
 drivers/input/input-compat.h | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)
drivers/input/input-compat.h +0 -3
index 148f66fe3205..0f25878d5fa2 100644
@@ -17,17 +17,7 @@
 
 #ifdef CONFIG_COMPAT
 
-/* Note to the author of this code: did it ever occur to
-   you why the ifdefs are needed? Think about it again. -AK */
-#if defined(CONFIG_X86_64) || defined(CONFIG_TILE)

drivers/input/input-compat.h line 21

Quoted 10 lines
drivers/input/input-compat.h +1 -8
-#  define INPUT_COMPAT_TEST is_compat_task()
-#elif defined(CONFIG_S390)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_31BIT)
-#elif defined(CONFIG_MIPS)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT_ADDR)
-#else
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT)
-#endif
+#define INPUT_COMPAT_TEST in_compat_syscall()
 

If we now have function that works on all arches I'd prefer if we used it directly instead of continuing using INPUT_COMPAT_TEST.

Thanks.

Signature · 1 line
Dmitry
#22 lore raw

Re: [PATCH v2 14/16] input: Redefine INPUT_COMPAT_TEST as in_compat_syscall()

Quoted 30 lines · Dmitry Torokhov

Hi Andy,

Quoted 27 lines · Andy Lutomirski

The input compat code should work like all other compat code: for 32-bit syscalls, use the 32-bit ABI and for 64-bit syscalls, use the 64-bit ABI. We have a helper for that (in_compat_syscall()): just use it.

---
 drivers/input/input-compat.h | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)
drivers/input/input-compat.h +0 -9
index 148f66fe3205..0f25878d5fa2 100644
@@ -17,17 +17,7 @@
 
 #ifdef CONFIG_COMPAT
 
-/* Note to the author of this code: did it ever occur to
-   you why the ifdefs are needed? Think about it again. -AK */
-#if defined(CONFIG_X86_64) || defined(CONFIG_TILE)
-#  define INPUT_COMPAT_TEST is_compat_task()
-#elif defined(CONFIG_S390)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_31BIT)
-#elif defined(CONFIG_MIPS)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT_ADDR)
-#else
Quoted 8 lines

drivers/input/input-compat.h line 21

Quoted 4 lines
drivers/input/input-compat.h +1 -2
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT)
-#endif
+#define INPUT_COMPAT_TEST in_compat_syscall()
 

If we now have function that works on all arches I'd prefer if we used it directly instead of continuing using INPUT_COMPAT_TEST.

I'll write a followup patch for that if you don't beat me to it.

--Andy

maintainer
#23 lore raw

Re: [PATCH v2 14/16] input: Redefine INPUT_COMPAT_TEST as in_compat_syscall()

Quoted 41 lines · Andy Lutomirski
Quoted 30 lines · Dmitry Torokhov

Hi Andy,

Quoted 27 lines · Andy Lutomirski

The input compat code should work like all other compat code: for 32-bit syscalls, use the 32-bit ABI and for 64-bit syscalls, use the 64-bit ABI. We have a helper for that (in_compat_syscall()): just use it.

---
 drivers/input/input-compat.h | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)
drivers/input/input-compat.h +0 -9
index 148f66fe3205..0f25878d5fa2 100644
@@ -17,17 +17,7 @@
 
 #ifdef CONFIG_COMPAT
 
-/* Note to the author of this code: did it ever occur to
-   you why the ifdefs are needed? Think about it again. -AK */
-#if defined(CONFIG_X86_64) || defined(CONFIG_TILE)
-#  define INPUT_COMPAT_TEST is_compat_task()
-#elif defined(CONFIG_S390)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_31BIT)
-#elif defined(CONFIG_MIPS)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT_ADDR)
-#else
Quoted 8 lines

drivers/input/input-compat.h line 21

Quoted 4 lines
drivers/input/input-compat.h +1 -2
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT)
-#endif
+#define INPUT_COMPAT_TEST in_compat_syscall()
 

If we now have function that works on all arches I'd prefer if we used it directly instead of continuing using INPUT_COMPAT_TEST.

Quoted 1 line

I'll write a followup patch for that if you don't beat me to it.

I promise I wont ;)

Signature · 1 line
Dmitry
#24 lore raw

Re: [PATCH v2 14/16] input: Redefine INPUT_COMPAT_TEST as in_compat_syscall()

Quoted 42 lines · On Wed, 27 Jan 2016 13:06:10 -0800 Dmitry Torokhov <[email protected]>…
Quoted 41 lines · Andy Lutomirski
Quoted 38 lines · Dmitry Torokhov

Hi Andy,

Quoted 21 lines · Andy Lutomirski

The input compat code should work like all other compat code: for 32-bit syscalls, use the 32-bit ABI and for 64-bit syscalls, use the 64-bit ABI. We have a helper for that (in_compat_syscall()): just use it.

---
 drivers/input/input-compat.h | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)
drivers/input/input-compat.h +0 -3
index 148f66fe3205..0f25878d5fa2 100644
@@ -17,17 +17,7 @@
 
 #ifdef CONFIG_COMPAT
 
-/* Note to the author of this code: did it ever occur to
-   you why the ifdefs are needed? Think about it again. -AK */
-#if defined(CONFIG_X86_64) || defined(CONFIG_TILE)

drivers/input/input-compat.h line 21

Quoted 10 lines
drivers/input/input-compat.h +1 -8
-#  define INPUT_COMPAT_TEST is_compat_task()
-#elif defined(CONFIG_S390)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_31BIT)
-#elif defined(CONFIG_MIPS)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT_ADDR)
-#else
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT)
-#endif
+#define INPUT_COMPAT_TEST in_compat_syscall()
 

If we now have function that works on all arches I'd prefer if we used it directly instead of continuing using INPUT_COMPAT_TEST.

Quoted 3 lines
Quoted 1 line

I'll write a followup patch for that if you don't beat me to it.

I promise I wont ;)

Well someone needs beating!

I'm prepping this patch for Linus now. I shall queue up the below for later.

From: Andrew Morton <[email protected]>
Subject: drivers/input: eliminate INPUT_COMPAT_TEST macro

INPUT_COMPAT_TEST became much simpler after "input: redefine INPUT_COMPAT_TEST as in_compat_syscall()" so we can cleanly eliminate it altogether.

Cc Dmitry Torokhov <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Andy Lutomirski <[email protected]>

---

 drivers/input/input-compat.c |    6 +++---
 drivers/input/input-compat.h |    4 +---
 drivers/input/input.c        |    2 +-
 drivers/input/misc/uinput.c  |    4 ++--
 4 files changed, 7 insertions(+), 9 deletions(-)
drivers/input/input-compat.h renamed from drivers/input/input-compat.h~a +1 -3
diff -puN drivers/input/input-compat.h~a drivers/input/input-compat.h
@@ -17,8 +17,6 @@
 
 #ifdef CONFIG_COMPAT
 
-#define INPUT_COMPAT_TEST in_compat_syscall()
-
 struct input_event_compat {
 	struct compat_timeval time;
 	__u16 type;
@@ -57,7 +55,7 @@ struct ff_effect_compat {
 
 static inline size_t input_event_size(void)
 {
-	return (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) ?
+	return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
 		sizeof(struct input_event_compat) : sizeof(struct input_event);
 }
 
drivers/input/misc/uinput.c renamed from drivers/input/misc/uinput.c~a +2 -2
diff -puN drivers/input/misc/uinput.c~a drivers/input/misc/uinput.c
@@ -664,7 +664,7 @@ struct uinput_ff_upload_compat {
 static int uinput_ff_upload_to_user(char __user *buffer,
 				    const struct uinput_ff_upload *ff_up)
 {
-	if (INPUT_COMPAT_TEST) {
+	if (in_compat_syscall()) {
 		struct uinput_ff_upload_compat ff_up_compat;
 
 		ff_up_compat.request_id = ff_up->request_id;
@@ -695,7 +695,7 @@ static int uinput_ff_upload_to_user(char
 static int uinput_ff_upload_from_user(const char __user *buffer,
 				      struct uinput_ff_upload *ff_up)
 {
-	if (INPUT_COMPAT_TEST) {
+	if (in_compat_syscall()) {
 		struct uinput_ff_upload_compat ff_up_compat;
 
 		if (copy_from_user(&ff_up_compat, buffer,
drivers/input/input-compat.c renamed from drivers/input/input-compat.c~a +3 -3
diff -puN drivers/input/input-compat.c~a drivers/input/input-compat.c
@@ -17,7 +17,7 @@
 int input_event_from_user(const char __user *buffer,
 			  struct input_event *event)
 {
-	if (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) {
+	if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) {
 		struct input_event_compat compat_event;
 
 		if (copy_from_user(&compat_event, buffer,
@@ -41,7 +41,7 @@ int input_event_from_user(const char __u
 int input_event_to_user(char __user *buffer,
 			const struct input_event *event)
 {
-	if (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) {
+	if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) {
 		struct input_event_compat compat_event;
 
 		compat_event.time.tv_sec = event->time.tv_sec;
@@ -65,7 +65,7 @@ int input_event_to_user(char __user *buf
 int input_ff_effect_from_user(const char __user *buffer, size_t size,
 			      struct ff_effect *effect)
 {
-	if (INPUT_COMPAT_TEST) {
+	if (in_compat_syscall()) {
 		struct ff_effect_compat *compat_effect;
 
 		if (size != sizeof(struct ff_effect_compat))
drivers/input/input.c renamed from drivers/input/input.c~a +1 -1
diff -puN drivers/input/input.c~a drivers/input/input.c
@@ -1015,7 +1015,7 @@ static int input_bits_to_string(char *bu
 {
 	int len = 0;
 
-	if (INPUT_COMPAT_TEST) {
+	if (in_compat_syscall()) {
 		u32 dword = bits >> 32;
 		if (dword || !skip_empty)
 			len += snprintf(buf, buf_size, "%x ", dword);

_

maintainer Acked-by
#25 lore raw

Re: [PATCH v2 14/16] input: Redefine INPUT_COMPAT_TEST as in_compat_syscall()

Quoted 44 lines · Andrew Morton
Quoted 42 lines · On Wed, 27 Jan 2016 13:06:10 -0800 Dmitry Torokhov <[email protected]>…
Quoted 41 lines · Andy Lutomirski
Quoted 38 lines · Dmitry Torokhov

Hi Andy,

Quoted 21 lines · Andy Lutomirski

The input compat code should work like all other compat code: for 32-bit syscalls, use the 32-bit ABI and for 64-bit syscalls, use the 64-bit ABI. We have a helper for that (in_compat_syscall()): just use it.

---
 drivers/input/input-compat.h | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)
drivers/input/input-compat.h +0 -3
index 148f66fe3205..0f25878d5fa2 100644
@@ -17,17 +17,7 @@
 
 #ifdef CONFIG_COMPAT
 
-/* Note to the author of this code: did it ever occur to
-   you why the ifdefs are needed? Think about it again. -AK */
-#if defined(CONFIG_X86_64) || defined(CONFIG_TILE)

drivers/input/input-compat.h line 21

Quoted 10 lines
drivers/input/input-compat.h +1 -8
-#  define INPUT_COMPAT_TEST is_compat_task()
-#elif defined(CONFIG_S390)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_31BIT)
-#elif defined(CONFIG_MIPS)
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT_ADDR)
-#else
-#  define INPUT_COMPAT_TEST test_thread_flag(TIF_32BIT)
-#endif
+#define INPUT_COMPAT_TEST in_compat_syscall()
 

If we now have function that works on all arches I'd prefer if we used it directly instead of continuing using INPUT_COMPAT_TEST.

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

I'll write a followup patch for that if you don't beat me to it.

I promise I wont ;)

Well someone needs beating!

I'm prepping this patch for Linus now. I shall queue up the below for later.

Thank you Andrew.

Quoted 4 lines

From: Andrew Morton <[email protected]>
Subject: drivers/input: eliminate INPUT_COMPAT_TEST macro

Quoted 8 lines

INPUT_COMPAT_TEST became much simpler after "input: redefine INPUT_COMPAT_TEST as in_compat_syscall()" so we can cleanly eliminate it altogether.

Cc Dmitry Torokhov <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Andy Lutomirski <[email protected]>
Quoted 94 lines

---

 drivers/input/input-compat.c |    6 +++---
 drivers/input/input-compat.h |    4 +---
 drivers/input/input.c        |    2 +-
 drivers/input/misc/uinput.c  |    4 ++--
 4 files changed, 7 insertions(+), 9 deletions(-)

diff -puN drivers/input/input-compat.h~a drivers/input/input-compat.h

drivers/input/input-compat.h renamed from drivers/input/input-compat.h~a +1 -3
@@ -17,8 +17,6 @@
 
 #ifdef CONFIG_COMPAT
 
-#define INPUT_COMPAT_TEST in_compat_syscall()
-
 struct input_event_compat {
 	struct compat_timeval time;
 	__u16 type;
@@ -57,7 +55,7 @@ struct ff_effect_compat {
 
 static inline size_t input_event_size(void)
 {
-	return (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) ?
+	return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
 		sizeof(struct input_event_compat) : sizeof(struct input_event);
 }
 

diff -puN drivers/input/misc/uinput.c~a drivers/input/misc/uinput.c

drivers/input/misc/uinput.c renamed from drivers/input/misc/uinput.c~a +2 -2
@@ -664,7 +664,7 @@ struct uinput_ff_upload_compat {
 static int uinput_ff_upload_to_user(char __user *buffer,
 				    const struct uinput_ff_upload *ff_up)
 {
-	if (INPUT_COMPAT_TEST) {
+	if (in_compat_syscall()) {
 		struct uinput_ff_upload_compat ff_up_compat;
 
 		ff_up_compat.request_id = ff_up->request_id;
@@ -695,7 +695,7 @@ static int uinput_ff_upload_to_user(char
 static int uinput_ff_upload_from_user(const char __user *buffer,
 				      struct uinput_ff_upload *ff_up)
 {
-	if (INPUT_COMPAT_TEST) {
+	if (in_compat_syscall()) {
 		struct uinput_ff_upload_compat ff_up_compat;
 
 		if (copy_from_user(&ff_up_compat, buffer,

diff -puN drivers/input/input-compat.c~a drivers/input/input-compat.c

drivers/input/input-compat.c renamed from drivers/input/input-compat.c~a +3 -3
@@ -17,7 +17,7 @@
 int input_event_from_user(const char __user *buffer,
 			  struct input_event *event)
 {
-	if (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) {
+	if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) {
 		struct input_event_compat compat_event;
 
 		if (copy_from_user(&compat_event, buffer,
@@ -41,7 +41,7 @@ int input_event_from_user(const char __u
 int input_event_to_user(char __user *buffer,
 			const struct input_event *event)
 {
-	if (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) {
+	if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) {
 		struct input_event_compat compat_event;
 
 		compat_event.time.tv_sec = event->time.tv_sec;
@@ -65,7 +65,7 @@ int input_event_to_user(char __user *buf
 int input_ff_effect_from_user(const char __user *buffer, size_t size,
 			      struct ff_effect *effect)
 {
-	if (INPUT_COMPAT_TEST) {
+	if (in_compat_syscall()) {
 		struct ff_effect_compat *compat_effect;
 
 		if (size != sizeof(struct ff_effect_compat))

diff -puN drivers/input/input.c~a drivers/input/input.c

drivers/input/input.c renamed from drivers/input/input.c~a +1 -1
@@ -1015,7 +1015,7 @@ static int input_bits_to_string(char *bu
 {
 	int len = 0;
 
-	if (INPUT_COMPAT_TEST) {
+	if (in_compat_syscall()) {
 		u32 dword = bits >> 32;
 		if (dword || !skip_empty)
 			len += snprintf(buf, buf_size, "%x ", dword);

_

Signature · 1 line
Dmitry
#26 lore raw

PATCH v2 15/16 uhid: Check write() bitness using in_compat_syscall

In reply to Andy Lutomirski

uhid changes the format expected in write() depending on bitness. It should check the syscall bitness directly.

---
 drivers/hid/uhid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hid/uhid.c +1 -1
index e094c572b86e..16b6f11a0700 100644
@@ -384,7 +384,7 @@ struct uhid_create_req_compat {
 static int uhid_event_from_user(const char __user *buffer, size_t len,
 				struct uhid_event *event)
 {
-	if (is_compat_task()) {
+	if (in_compat_syscall()) {
 		u32 type;
 
 		if (get_user(type, buffer))
Signature · 1 line
2.5.0
#27 lore raw

PATCH v2 16/16 x86/compat: Remove is_compat_task

In reply to Andy Lutomirski

x86's is_compat_task always checked the current syscall type, not the task type. It has no non-arch users any more, so just remove it to avoid confusion.

On x86, nothing should really be checking the task ABI. There are legitimate users for the syscall ABI and for the mm ABI.

---
 arch/x86/include/asm/compat.h | 3 ++-
 arch/x86/include/asm/ftrace.h | 2 +-
 arch/x86/kernel/process_64.c  | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)
arch/x86/include/asm/compat.h +2 -1
index acdee09228b3..ebb102e1bbc7 100644
@@ -316,9 +316,10 @@ static inline bool is_x32_task(void)
 	return false;
 }
 
-static inline bool is_compat_task(void)
+static inline bool in_compat_syscall(void)
 {
 	return is_ia32_task() || is_x32_task();
 }
+#define in_compat_syscall in_compat_syscall	/* override the generic impl */
 
 #endif /* _ASM_X86_COMPAT_H */
arch/x86/include/asm/ftrace.h +1 -1
index 24938852db30..21b66dbf3601 100644
@@ -58,7 +58,7 @@ int ftrace_int3_handler(struct pt_regs *regs);
 #define ARCH_TRACE_IGNORE_COMPAT_SYSCALLS 1
 static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs)
 {
-	if (is_compat_task())
+	if (in_compat_syscall())
 		return true;
 	return false;
 }
arch/x86/kernel/process_64.c +1 -1
index b9d99e0f82c4..71a18a22ae2f 100644
@@ -476,7 +476,7 @@ void set_personality_ia32(bool x32)
 		if (current->mm)
 			current->mm->context.ia32_compat = TIF_X32;
 		current->personality &= ~READ_IMPLIES_EXEC;
-		/* is_compat_task() uses the presence of the x32
+		/* in_compat_syscall() uses the presence of the x32
 		   syscall bit flag to determine compat status */
 		current_thread_info()->status &= ~TS_COMPAT;
 	} else {
Signature · 1 line
2.5.0