compat: Introduce and use in_compat_syscall
Subsystems (22)
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
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
@@ -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
PATCH v2 2/16 sparc/compat: Provide an accurate in_compat_syscall implementation
In reply to Andy LutomirskiOn 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
@@ -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
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_taskreturns true in 32-bit tasks, which does not necessarily imply that the current syscall is 32-bit.Provide an
in_compat_syscallimplementation 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_taskchecks 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_taskchecks the syscall type, not the task type.Signed-off-by Andy Lutomirski <[email protected]>
Re: [PATCH v2 02/16] sparc/compat: Provide an accurate in_compat_syscall implementation
In reply to Andy LutomirskiQuoted 23 lines · Andy Lutomirski
On sparc64 compat-enabled kernels, any task can make 32-bit and 64-bit syscalls.
is_compat_taskreturns true in 32-bit tasks, which does not necessarily imply that the current syscall is 32-bit.Provide an
in_compat_syscallimplementation that checks whether the current syscall is compat.As far as I know, sparc is the only architecture on which
is_compat_taskchecks 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_taskchecks the syscall type, not the task type.Signed-off-by Andy Lutomirski <[email protected]>--- arch/sparc/include/asm/compat.h | 6 ++++++ 1 file changed, 6 insertions(+)arch/sparc/include/asm/compat.h +0
-0
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
David Miller
maintainerRe: [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
0x110comes 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_TRAPtl0_oldlinux64: LINUX_64BIT_SYSCALL_TRAP
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 +0100Quoted 2 lines
Could you please add a comment about where
0x110comes 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
0x110and see:
tl0_resv10e: BTRAP(0x10e) BTRAP(0x10f)tl0_linux32:LINUX_32BIT_SYSCALL_TRAPtl0_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
Andy Lutomirski
reviewerRe: [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 +0100Quoted 2 lines
Could you please add a comment about where
0x110comes 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
0x110and 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
David Miller
maintainerRe: [PATCH v2 02/16] sparc/compat: Provide an accurate in_compat_syscall implementation
In reply to Sam RavnborgFrom: 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 +0100Quoted 2 lines
Could you please add a comment about where
0x110comes 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
0x110and see:
tl0_resv10e: BTRAP(0x10e) BTRAP(0x10f)tl0_linux32:LINUX_32BIT_SYSCALL_TRAPtl0_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.
PATCH v2 3/16 sparc/syscall: Fix syscall_get_arch
In reply to Andy LutomirskiSparc'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
@@ -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
PATCH v2 4/16 seccomp: Check in_compat_syscall, not is_compat_task, in strict mode
In reply to Andy LutomirskiSeccomp 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
@@ -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
PATCH v2 5/16 ptrace: in PEEK_SIGINFO, check syscall bitness, not task bitness
In reply to Andy LutomirskiUsers 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
@@ -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
PATCH v2 6/16 auditsc: For seccomp events, log syscall compat state using in_compat_syscall
In reply to Andy LutomirskiExcept 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
@@ -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
PATCH v2 7/16 staging/lustre: Switch from is_compat_task to in_compat_syscall
In reply to Andy LutomirskiAFAICT, 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
@@ -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
PATCH v2 8/16 ext4: In ext4_dir_llseek, check syscall bitness directly
In reply to Andy Lutomirskiext4 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
@@ -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
PATCH v2 9/16 net/sctp: Use in_compat_syscall for sctp_getsockopt_connectx3
In reply to Andy LutomirskiSCTP 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
@@ -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
PATCH v2 10/16 net/xfrm_user: Use in_compat_syscall to deny compat syscalls
In reply to Andy LutomirskiThe 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
@@ -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
PATCH v2 11/16 firewire: Use in_compat_syscall to check ioctl compatness
In reply to Andy LutomirskiFirewire 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
@@ -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
PATCH v2 12/16 efivars: Use in_compat_syscall to check for compat callers
In reply to Andy LutomirskiThis 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
@@ -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
PATCH v2 13/16 amdkfd: Use in_compat_syscall to check open() caller type
In reply to Andy Lutomirskiamdkfd 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
@@ -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
@@ -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
PATCH v2 14/16 input: Redefine INPUT_COMPAT_TEST as in_compat_syscall()
In reply to Andy LutomirskiThe 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
@@ -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
Dmitry Torokhov
maintainerRe: [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.Signed-off-by Andy Lutomirski <[email protected]>--- drivers/input/input-compat.h | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)drivers/input/input-compat.h +0
-3@@ -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
Andy Lutomirski
reviewerRe: [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.Signed-off-by Andy Lutomirski <[email protected]>--- drivers/input/input-compat.h | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)drivers/input/input-compat.h +0
-9@@ -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
Dmitry Torokhov
maintainerRe: [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.Signed-off-by Andy Lutomirski <[email protected]>--- drivers/input/input-compat.h | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)drivers/input/input-compat.h +0
-9@@ -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
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.Signed-off-by Andy Lutomirski <[email protected]>--- drivers/input/input-compat.h | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)drivers/input/input-compat.h +0
-3@@ -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.
---
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
@@ -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
@@ -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(charstatic 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
@@ -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 __uint 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 *bufint 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
@@ -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);
_
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.Signed-off-by Andy Lutomirski <[email protected]>--- drivers/input/input-compat.h | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)drivers/input/input-compat.h +0
-3@@ -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: eliminateINPUT_COMPAT_TESTmacro
Quoted 8 lines
INPUT_COMPAT_TESTbecame much simpler after "input: redefineINPUT_COMPAT_TESTasin_compat_syscall()" so we can cleanly eliminate it altogether.Cc Dmitry Torokhov <[email protected]>Cc Andy Lutomirski <[email protected]>Cc Andy Lutomirski <[email protected]>Signed-off-by Andrew Morton <[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~adrivers/input/input-compat.hdrivers/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~adrivers/input/misc/uinput.cdrivers/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(charstatic 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~adrivers/input/input-compat.cdrivers/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 __uint 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 *bufint 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~adrivers/input/input.cdrivers/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
PATCH v2 15/16 uhid: Check write() bitness using in_compat_syscall
In reply to Andy Lutomirskiuhid 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
@@ -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
PATCH v2 16/16 x86/compat: Remove is_compat_task
In reply to Andy Lutomirskix86'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
@@ -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
@@ -58,7 +58,7 @@ int ftrace_int3_handler(struct pt_regs *regs);#define ARCH_TRACE_IGNORE_COMPAT_SYSCALLS 1static 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
@@ -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