linux-kernel - Re: [PATCH] Syscall arguments are unsigned long (full registers)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <C22BF648-D579-4B23-8BF6-C18033433F22@zytor.com>
Date:	Mon, 04 Jul 2016 11:27:25 -0700
From:	"H. Peter Anvin" <hpa@...or.com>
To:	"Tautschnig, Michael" <tautschn@...zon.co.uk>,
	"x86@...nel.org" <x86@...nel.org>,
	"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	Jaswinder Singh <jaswinder@...radead.org>,
	Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH] Syscall arguments are unsigned long (full registers)

On July 4, 2016 6:52:58 AM PDT, "Tautschnig, Michael" <tautschn@...zon.co.uk> wrote:
>All syscall arguments are passed in as types of the same byte size as
>unsigned long (width of full registers). Using a smaller type without a
>cast may result in losing bits of information. In all other instances
>apart from the ones fixed by the patch the code explicitly introduces
>type casts (using, e.g., SYSCALL_DEFINE1).
>
>While goto-cc reported these problems at build time, it is noteworthy
>that the calling conventions specified in the System V AMD64 ABI do
>ensure that parameters 1-6 are passed via registers, thus there is no
>implied risk of misaligned stack access.
>
>Signed-off-by: Michael Tautschnig <tautschn@...zon.com>
>Cc: Thomas Gleixner <tglx@...utronix.de>
>Cc: Ingo Molnar <mingo@...hat.com>
>Cc: H. Peter Anvin <hpa@...or.com>
>Cc: Jaswinder Singh <jaswinder@...radead.org>
>Cc: Andi Kleen <ak@...ux.intel.com>
>---
> arch/x86/include/asm/syscalls.h | 4 ++--
> arch/x86/kernel/ioport.c        | 2 +-
> arch/x86/kernel/process_64.c    | 2 +-
> include/linux/syscalls.h        | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
>diff --git a/arch/x86/include/asm/syscalls.h
>b/arch/x86/include/asm/syscalls.h
>index 91dfcaf..7dc3161 100644
>--- a/arch/x86/include/asm/syscalls.h
>+++ b/arch/x86/include/asm/syscalls.h
>@@ -17,7 +17,7 @@
>
> /* Common in X86_32 and X86_64 */
> /* kernel/ioport.c */
>-asmlinkage long sys_ioperm(unsigned long, unsigned long, int);
>+asmlinkage long sys_ioperm(unsigned long, unsigned long, unsigned
>long);
> asmlinkage long sys_iopl(unsigned int);
>
> /* kernel/ldt.c */
>@@ -45,7 +45,7 @@ asmlinkage long sys_vm86(unsigned long, unsigned
>long);
>
> /* X86_64 only */
> /* kernel/process_64.c */
>-asmlinkage long sys_arch_prctl(int, unsigned long);
>+asmlinkage long sys_arch_prctl(unsigned long, unsigned long);
>
> /* kernel/sys_x86_64.c */
> asmlinkage long sys_mmap(unsigned long, unsigned long, unsigned long,
>diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
>index 589b319..ae8ce91 100644
>--- a/arch/x86/kernel/ioport.c
>+++ b/arch/x86/kernel/ioport.c
>@@ -20,7 +20,7 @@
> /*
>  * this changes the io permissions bitmap in the current task.
>  */
>-asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int
>turn_on)
>+asmlinkage long sys_ioperm(unsigned long from, unsigned long num,
>unsigned long turn_on)
> {
> 	struct thread_struct *t = &current->thread;
> 	struct tss_struct *tss;
>diff --git a/arch/x86/kernel/process_64.c
>b/arch/x86/kernel/process_64.c
>index 6e789ca..a4ec3e3 100644
>--- a/arch/x86/kernel/process_64.c
>+++ b/arch/x86/kernel/process_64.c
>@@ -585,7 +585,7 @@ long do_arch_prctl(struct task_struct *task, int
>code, unsigned long addr)
> 	return ret;
> }
>
>-long sys_arch_prctl(int code, unsigned long addr)
>+long sys_arch_prctl(unsigned long code, unsigned long addr)
> {
> 	return do_arch_prctl(current, code, addr);
> }
>diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
>index d022390..ca3c03d 100644
>--- a/include/linux/syscalls.h
>+++ b/include/linux/syscalls.h
>@@ -492,7 +492,7 @@ asmlinkage long sys_pipe2(int __user *fildes, int
>flags);
> asmlinkage long sys_dup(unsigned int fildes);
> asmlinkage long sys_dup2(unsigned int oldfd, unsigned int newfd);
>asmlinkage long sys_dup3(unsigned int oldfd, unsigned int newfd, int
>flags);
>-asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int
>on);
>+asmlinkage long sys_ioperm(unsigned long from, unsigned long num,
>unsigned long on);
> asmlinkage long sys_ioctl(unsigned int fd, unsigned int cmd,
> 				unsigned long arg);
> asmlinkage long sys_flock(unsigned int fd, unsigned int cmd);
>--
>2.7.3.AMZN

Wrong.  Syscall arguments aren't necessarily full registers, and on x86 truncation is already done by the callee, so we don't need any special handing.  Some other architectures have other constraints.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.