[tip:x86/urgent] x86/sigreturn: Use SYSCALL_DEFINE0

[tip-bot for Tautschnig, Michael]

Commit-ID:  1227b344de283f96c41de02cde4b26d5290f5302
Gitweb: https://git.kernel.org/tip/1227b344de283f96c41de02cde4b26d5290f5302
Author: Tautschnig, Michael <tauts...@amazon.co.uk>
AuthorDate: Wed, 14 Mar 2018 09:41:42 +
Committer:  Thomas Gleixner <t...@linutronix.de>
CommitDate: Wed, 14 Mar 2018 21:43:09 +0100

x86/sigreturn: Use SYSCALL_DEFINE0

All definitions of syscalls in x86 except for those patched here have
already been using the appropriate SYSCALL_DEFINE*.

Signed-off-by: Michael Tautschnig <tauts...@amazon.com>
Signed-off-by: Thomas Gleixner <t...@linutronix.de>
Cc: Andi Kleen <a...@linux.intel.com>
Cc: linux-...@vger.kernel.org
Cc: Dominik Brodowski <li...@dominikbrodowski.net>
Cc: Jaswinder Singh <jaswin...@infradead.org>
Cc: "linux-...@vger.kernel.org"
Cc: "x...@kernel.org"
Link: https://lkml.kernel.org/r/9c9981f4-0e5e-42a7-8788-70f3dead2...@amazon.com

---
 arch/x86/kernel/signal.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 4cdc0b27ec82..da270b95fe4d 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -601,7 +602,7 @@ static int x32_setup_rt_frame(struct ksignal *ksig,
  * Do a signal return; undo the signal stack.
  */
 #ifdef CONFIG_X86_32
-asmlinkage unsigned long sys_sigreturn(void)
+SYSCALL_DEFINE0(sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct sigframe __user *frame;
@@ -633,7 +634,7 @@ badframe:
 }
 #endif /* CONFIG_X86_32 */
 
-asmlinkage long sys_rt_sigreturn(void)
+SYSCALL_DEFINE0(rt_sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe __user *frame;

[tip:x86/urgent] x86/sigreturn: Use SYSCALL_DEFINE0

[tip-bot for Tautschnig, Michael]

Commit-ID:  1227b344de283f96c41de02cde4b26d5290f5302
Gitweb: https://git.kernel.org/tip/1227b344de283f96c41de02cde4b26d5290f5302
Author: Tautschnig, Michael 
AuthorDate: Wed, 14 Mar 2018 09:41:42 +
Committer:  Thomas Gleixner 
CommitDate: Wed, 14 Mar 2018 21:43:09 +0100

x86/sigreturn: Use SYSCALL_DEFINE0

All definitions of syscalls in x86 except for those patched here have
already been using the appropriate SYSCALL_DEFINE*.

Signed-off-by: Michael Tautschnig 
Signed-off-by: Thomas Gleixner 
Cc: Andi Kleen 
Cc: linux-...@vger.kernel.org
Cc: Dominik Brodowski 
Cc: Jaswinder Singh 
Cc: "linux-...@vger.kernel.org"
Cc: "x...@kernel.org"
Link: https://lkml.kernel.org/r/9c9981f4-0e5e-42a7-8788-70f3dead2...@amazon.com

---
 arch/x86/kernel/signal.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 4cdc0b27ec82..da270b95fe4d 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -601,7 +602,7 @@ static int x32_setup_rt_frame(struct ksignal *ksig,
  * Do a signal return; undo the signal stack.
  */
 #ifdef CONFIG_X86_32
-asmlinkage unsigned long sys_sigreturn(void)
+SYSCALL_DEFINE0(sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct sigframe __user *frame;
@@ -633,7 +634,7 @@ badframe:
 }
 #endif /* CONFIG_X86_32 */
 
-asmlinkage long sys_rt_sigreturn(void)
+SYSCALL_DEFINE0(rt_sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe __user *frame;

Re: [PATCH] x86: always use SYSCALL_DEFINE*

[Tautschnig, Michael]

Hi Dominik,

> On 14 Mar 2018, at 05:48, Dominik Brodowski  
> wrote:
> [...]
> ... the ioperm change is already in mainline (did an equivalent change a
> couple of days ago), but the sigreturn/rt_sigreturn changes still seem
> useful. Could you send a fresh patch with just these two changes; and -- if
> the x86 maintainers agree -- I will push it with my syscall-related changes?

Thanks for all this cleanup work. I have posted the {rt_,}sigreturn changes as
"[PATCH] x86/sigreturn: use SYSCALL_DEFINE0".

Best,
Michael






Amazon Web Services UK Limited. Registered in England and Wales with 
registration number 08650665 with its registered office at 1 Principal Place, 
Worship Street, London, EC2A 2FA, United Kingdom.

Re: [PATCH] x86: always use SYSCALL_DEFINE*

[Tautschnig, Michael]

Hi Dominik,

> On 14 Mar 2018, at 05:48, Dominik Brodowski  
> wrote:
> [...]
> ... the ioperm change is already in mainline (did an equivalent change a
> couple of days ago), but the sigreturn/rt_sigreturn changes still seem
> useful. Could you send a fresh patch with just these two changes; and -- if
> the x86 maintainers agree -- I will push it with my syscall-related changes?

Thanks for all this cleanup work. I have posted the {rt_,}sigreturn changes as
"[PATCH] x86/sigreturn: use SYSCALL_DEFINE0".

Best,
Michael






Amazon Web Services UK Limited. Registered in England and Wales with 
registration number 08650665 with its registered office at 1 Principal Place, 
Worship Street, London, EC2A 2FA, United Kingdom.

[PATCH] x86/sigreturn: use SYSCALL_DEFINE0

[Tautschnig, Michael]

All definitions of syscalls in x86 except for those patched here have
already been using the appropriate SYSCALL_DEFINE*.

Signed-off-by: Michael Tautschnig 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: Jaswinder Singh 
Cc: Andi Kleen 
Cc: Dominik Brodowski 
---
 arch/x86/kernel/signal.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index b9e00e8..40ba242 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -601,7 +602,7 @@ static int x32_setup_rt_frame(struct ksignal *ksig,
  * Do a signal return; undo the signal stack.
  */
 #ifdef CONFIG_X86_32
-asmlinkage unsigned long sys_sigreturn(void)
+SYSCALL_DEFINE0(sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct sigframe __user *frame;
@@ -633,7 +634,7 @@ asmlinkage unsigned long sys_sigreturn(void)
 }
 #endif /* CONFIG_X86_32 */

-asmlinkage long sys_rt_sigreturn(void)
+SYSCALL_DEFINE0(rt_sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe __user *frame;
--
2.7.3.AMZN



Amazon Web Services UK Limited. Registered in England and Wales with 
registration number 08650665 with its registered office at 1 Principal Place, 
Worship Street, London, EC2A 2FA, United Kingdom.

[PATCH] x86/sigreturn: use SYSCALL_DEFINE0

[Tautschnig, Michael]

All definitions of syscalls in x86 except for those patched here have
already been using the appropriate SYSCALL_DEFINE*.

Signed-off-by: Michael Tautschnig 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: Jaswinder Singh 
Cc: Andi Kleen 
Cc: Dominik Brodowski 
---
 arch/x86/kernel/signal.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index b9e00e8..40ba242 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -601,7 +602,7 @@ static int x32_setup_rt_frame(struct ksignal *ksig,
  * Do a signal return; undo the signal stack.
  */
 #ifdef CONFIG_X86_32
-asmlinkage unsigned long sys_sigreturn(void)
+SYSCALL_DEFINE0(sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct sigframe __user *frame;
@@ -633,7 +634,7 @@ asmlinkage unsigned long sys_sigreturn(void)
 }
 #endif /* CONFIG_X86_32 */

-asmlinkage long sys_rt_sigreturn(void)
+SYSCALL_DEFINE0(rt_sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe __user *frame;
--
2.7.3.AMZN



Amazon Web Services UK Limited. Registered in England and Wales with 
registration number 08650665 with its registered office at 1 Principal Place, 
Worship Street, London, EC2A 2FA, United Kingdom.

[PATCH v2] x86: always use SYSCALL_DEFINE*

[Tautschnig, Michael]

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. SYSCALL_DEFINE* introduce
adequate type casts. All definitions of syscalls in x86 except for those
patched here have already been using the appropriate SYSCALL_DEFINE*.

Signed-off-by: Michael Tautschnig 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: Jaswinder Singh 
Cc: Andi Kleen 
---
 arch/x86/kernel/ioport.c | 3 ++-
 arch/x86/kernel/signal.c | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 2f72330..7db3d65 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -23,7 +23,8 @@
 /*
  * this changes the io permissions bitmap in the current task.
  */
-asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
+SYSCALL_DEFINE3(ioperm, unsigned long, from, unsigned long, num, int,
+   turn_on)
 {
struct thread_struct *t = >thread;
struct tss_struct *tss;
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index b9e00e8..40ba242 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -601,7 +602,7 @@ static int x32_setup_rt_frame(struct ksignal *ksig,
  * Do a signal return; undo the signal stack.
  */
 #ifdef CONFIG_X86_32
-asmlinkage unsigned long sys_sigreturn(void)
+SYSCALL_DEFINE0(sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct sigframe __user *frame;
@@ -633,7 +634,7 @@ asmlinkage unsigned long sys_sigreturn(void)
 }
 #endif /* CONFIG_X86_32 */

-asmlinkage long sys_rt_sigreturn(void)
+SYSCALL_DEFINE0(rt_sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe __user *frame;
--
2.7.3.AMZN



Amazon Web Services UK Limited. Registered in England and Wales with 
registration number 08650665 with its registered office at 1 Principal Place, 
Worship Street, London, EC2A 2FA, United Kingdom.

[PATCH v2] x86: always use SYSCALL_DEFINE*

[Tautschnig, Michael]

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. SYSCALL_DEFINE* introduce
adequate type casts. All definitions of syscalls in x86 except for those
patched here have already been using the appropriate SYSCALL_DEFINE*.

Signed-off-by: Michael Tautschnig 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: Jaswinder Singh 
Cc: Andi Kleen 
---
 arch/x86/kernel/ioport.c | 3 ++-
 arch/x86/kernel/signal.c | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 2f72330..7db3d65 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -23,7 +23,8 @@
 /*
  * this changes the io permissions bitmap in the current task.
  */
-asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
+SYSCALL_DEFINE3(ioperm, unsigned long, from, unsigned long, num, int,
+   turn_on)
 {
struct thread_struct *t = >thread;
struct tss_struct *tss;
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index b9e00e8..40ba242 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -601,7 +602,7 @@ static int x32_setup_rt_frame(struct ksignal *ksig,
  * Do a signal return; undo the signal stack.
  */
 #ifdef CONFIG_X86_32
-asmlinkage unsigned long sys_sigreturn(void)
+SYSCALL_DEFINE0(sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct sigframe __user *frame;
@@ -633,7 +634,7 @@ asmlinkage unsigned long sys_sigreturn(void)
 }
 #endif /* CONFIG_X86_32 */

-asmlinkage long sys_rt_sigreturn(void)
+SYSCALL_DEFINE0(rt_sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe __user *frame;
--
2.7.3.AMZN



Amazon Web Services UK Limited. Registered in England and Wales with 
registration number 08650665 with its registered office at 1 Principal Place, 
Worship Street, London, EC2A 2FA, United Kingdom.

Re: [PATCH] x86: always use SYSCALL_DEFINE*

[Tautschnig, Michael]

On 10 Mar 2018, at 20:55, Tautschnig, Michael <tauts...@amazon.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. SYSCALL_DEFINE* introduce
> adequate type casts. All definitions of syscalls in x86 except for those
> patched here have already been using the appropriate SYSCALL_DEFINE*.
[...]

Additional context: I had previously made an attempt to ensure type
consistency of sys_ioperm as "Syscall arguments are unsigned long (full
registers)" (https://lkml.org/lkml/2016/7/4/336). I hope the new proposal
is more acceptable.

Best,
Michael



Amazon Web Services UK Limited. Registered in England and Wales with 
registration number 08650665 with its registered office at 1 Principal Place, 
Worship Street, London, EC2A 2FA, United Kingdom.

Re: [PATCH] x86: always use SYSCALL_DEFINE*

[Tautschnig, Michael]

On 10 Mar 2018, at 20:55, Tautschnig, Michael  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. SYSCALL_DEFINE* introduce
> adequate type casts. All definitions of syscalls in x86 except for those
> patched here have already been using the appropriate SYSCALL_DEFINE*.
[...]

Additional context: I had previously made an attempt to ensure type
consistency of sys_ioperm as "Syscall arguments are unsigned long (full
registers)" (https://lkml.org/lkml/2016/7/4/336). I hope the new proposal
is more acceptable.

Best,
Michael



Amazon Web Services UK Limited. Registered in England and Wales with 
registration number 08650665 with its registered office at 1 Principal Place, 
Worship Street, London, EC2A 2FA, United Kingdom.

[PATCH] x86: always use SYSCALL_DEFINE*

[Tautschnig, Michael]

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. SYSCALL_DEFINE* introduce
adequate type casts. All definitions of syscalls in x86 except for those
patched here have already been using the appropriate SYSCALL_DEFINE*.

Signed-off-by: Michael Tautschnig 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: Jaswinder Singh 
Cc: Andi Kleen 
---
 arch/x86/kernel/ioport.c | 3 ++-
 arch/x86/kernel/signal.c | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 2f72330..d98b2a3 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -23,7 +23,8 @@
 /*
  * this changes the io permissions bitmap in the current task.
  */
-asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
+SYSCALL_DEFINE3(sys_ioperm, unsigned long, from, unsigned long, num, int,
+   turn_on)
 {
struct thread_struct *t = >thread;
struct tss_struct *tss;
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index b9e00e8..40ba242 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -601,7 +602,7 @@ static int x32_setup_rt_frame(struct ksignal *ksig,
  * Do a signal return; undo the signal stack.
  */
 #ifdef CONFIG_X86_32
-asmlinkage unsigned long sys_sigreturn(void)
+SYSCALL_DEFINE0(sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct sigframe __user *frame;
@@ -633,7 +634,7 @@ asmlinkage unsigned long sys_sigreturn(void)
 }
 #endif /* CONFIG_X86_32 */

-asmlinkage long sys_rt_sigreturn(void)
+SYSCALL_DEFINE0(rt_sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe __user *frame;
--
2.7.3.AMZN



Amazon Web Services UK Limited. Registered in England and Wales with 
registration number 08650665 with its registered office at 1 Principal Place, 
Worship Street, London, EC2A 2FA, United Kingdom.

[PATCH] x86: always use SYSCALL_DEFINE*

[Tautschnig, Michael]

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. SYSCALL_DEFINE* introduce
adequate type casts. All definitions of syscalls in x86 except for those
patched here have already been using the appropriate SYSCALL_DEFINE*.

Signed-off-by: Michael Tautschnig 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: Jaswinder Singh 
Cc: Andi Kleen 
---
 arch/x86/kernel/ioport.c | 3 ++-
 arch/x86/kernel/signal.c | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 2f72330..d98b2a3 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -23,7 +23,8 @@
 /*
  * this changes the io permissions bitmap in the current task.
  */
-asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
+SYSCALL_DEFINE3(sys_ioperm, unsigned long, from, unsigned long, num, int,
+   turn_on)
 {
struct thread_struct *t = >thread;
struct tss_struct *tss;
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index b9e00e8..40ba242 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -601,7 +602,7 @@ static int x32_setup_rt_frame(struct ksignal *ksig,
  * Do a signal return; undo the signal stack.
  */
 #ifdef CONFIG_X86_32
-asmlinkage unsigned long sys_sigreturn(void)
+SYSCALL_DEFINE0(sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct sigframe __user *frame;
@@ -633,7 +634,7 @@ asmlinkage unsigned long sys_sigreturn(void)
 }
 #endif /* CONFIG_X86_32 */

-asmlinkage long sys_rt_sigreturn(void)
+SYSCALL_DEFINE0(rt_sigreturn)
 {
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe __user *frame;
--
2.7.3.AMZN



Amazon Web Services UK Limited. Registered in England and Wales with 
registration number 08650665 with its registered office at 1 Principal Place, 
Worship Street, London, EC2A 2FA, United Kingdom.

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

[Tautschnig, Michael]

> On 4 Jul 2016, at 20:27, H. Peter Anvin <h...@zytor.com> wrote:
> 
> On July 4, 2016 6:52:58 AM PDT, "Tautschnig, Michael" <tauts...@amazon.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.
>> 
>> 
[...]
> 
> 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. 

Ok - I'm assuming I have thus misunderstood 
eb974c62565072e10c1422eb3205f5b611dd99a1 ? Supposedly all those SYSCALL_DEFINEx 
are required for other architectures only?

Best,
Michael

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

[Tautschnig, Michael]

> On 4 Jul 2016, at 20:27, H. Peter Anvin  wrote:
> 
> On July 4, 2016 6:52:58 AM PDT, "Tautschnig, Michael"  
> 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.
>> 
>> 
[...]
> 
> 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. 

Ok - I'm assuming I have thus misunderstood 
eb974c62565072e10c1422eb3205f5b611dd99a1 ? Supposedly all those SYSCALL_DEFINEx 
are required for other architectures only?

Best,
Michael

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

[Tautschnig, Michael]

> On 4 Jul 2016, at 16:59, Arnd Bergmann <a...@arndb.de> wrote:
> 
> On Monday, July 4, 2016 2:47:10 PM CEST Tautschnig, Michael wrote:
>> Thanks a lot for the immediate feedback.
>> 
>>> On 4 Jul 2016, at 16:28, Andi Kleen <a...@linux.intel.com> wrote:
>>> 
>>> On Mon, Jul 04, 2016 at 01:52:58PM +, Tautschnig, Michael 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.
>>> 
>>> Does this actually fix anything?
>>> 
>> 
>> It will ensure the behaviour on 32 and 64-bit systems is consistent, i.e.,
>> no truncation occurs. This is to ensure that future uses of these syscalls
   ^^^ no *hidden*

>> do not face surprises.
>> 

[...]
> This is the same truncation that we do with SYSCALL_DEFINE2(),
> clearing the top 32 bits of the 'code' parameter to ensure that
> user space doesn't pass data unexpectedly.
> 
> That change seems reasonable, but why not just use SYSCALL_DEFINE2()
> directly for consistency with the other syscalls?
> 

Happy to provide such an updated patch; Andi seemed less confident this should
be going ahead?

Best,
Michael

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

[Tautschnig, Michael]

> On 4 Jul 2016, at 16:59, Arnd Bergmann  wrote:
> 
> On Monday, July 4, 2016 2:47:10 PM CEST Tautschnig, Michael wrote:
>> Thanks a lot for the immediate feedback.
>> 
>>> On 4 Jul 2016, at 16:28, Andi Kleen  wrote:
>>> 
>>> On Mon, Jul 04, 2016 at 01:52:58PM +, Tautschnig, Michael 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.
>>> 
>>> Does this actually fix anything?
>>> 
>> 
>> It will ensure the behaviour on 32 and 64-bit systems is consistent, i.e.,
>> no truncation occurs. This is to ensure that future uses of these syscalls
   ^^^ no *hidden*

>> do not face surprises.
>> 

[...]
> This is the same truncation that we do with SYSCALL_DEFINE2(),
> clearing the top 32 bits of the 'code' parameter to ensure that
> user space doesn't pass data unexpectedly.
> 
> That change seems reasonable, but why not just use SYSCALL_DEFINE2()
> directly for consistency with the other syscalls?
> 

Happy to provide such an updated patch; Andi seemed less confident this should
be going ahead?

Best,
Michael

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

[Tautschnig, Michael]

Thanks a lot for the immediate feedback.

> On 4 Jul 2016, at 16:28, Andi Kleen <a...@linux.intel.com> wrote:
> 
> On Mon, Jul 04, 2016 at 01:52:58PM +0000, Tautschnig, Michael 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.
> 
> Does this actually fix anything?
> 

It will ensure the behaviour on 32 and 64-bit systems is consistent, i.e.,
no truncation occurs. This is to ensure that future uses of these syscalls
do not face surprises.

> It seems a big dangerous to me, potentially breaking some existing
> binaries that rely on these arguments being truncated.
> 

Would an analysis of all current call sites be of help? It seems impossible
to tell whether any modules outside the kernel tree using this functionality
rely on the (seemingly broken) behaviour.

Of course I could also provide a patch that introduces explicit type casts
to document the truncation.

Best,
Michael

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

[Tautschnig, Michael]

Thanks a lot for the immediate feedback.

> On 4 Jul 2016, at 16:28, Andi Kleen  wrote:
> 
> On Mon, Jul 04, 2016 at 01:52:58PM +, Tautschnig, Michael 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.
> 
> Does this actually fix anything?
> 

It will ensure the behaviour on 32 and 64-bit systems is consistent, i.e.,
no truncation occurs. This is to ensure that future uses of these syscalls
do not face surprises.

> It seems a big dangerous to me, potentially breaking some existing
> binaries that rely on these arguments being truncated.
> 

Would an analysis of all current call sites be of help? It seems impossible
to tell whether any modules outside the kernel tree using this functionality
rely on the (seemingly broken) behaviour.

Of course I could also provide a patch that introduces explicit type casts
to document the truncation.

Best,
Michael

[PATCH] scripts: Fix size mismatch of kexec_purgatory_size

[Tautschnig, Michael]

bin2c is used to create a valid C file out of a binary file where two
symbols will be globally defined:  and _size.  is
passed as the first parameter of the host binary.

Building using goto-cc reported that the purgatory binary code (the only
current user of this utility) declares kexec_purgatory_size as 'size_t'
where bin2c generate _size to be 'int' so in a 64-bit host where
sizeof(size_t) > sizeof(int) this type mismatch will always yield the
wrong value for big-endian architectures while for little-endian it will
be wrong if the object laid in memory directly after
kexec_purgatory_size contains non-zero value at the time of reading.

This commit changes _size to be size_t instead.

Note:

Another way to fix the problem is to change the type of
kexec_purgatory_size to be 'int' as there's this check in code:
(kexec_purgatory_size <= 0)

Signed-off-by: Michael Tautschnig 
Cc: Michal Marek 
Cc: Vivek Goyal 
---
 scripts/basic/bin2c.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/basic/bin2c.c b/scripts/basic/bin2c.c
index af187e6..c3d7eef 100644
--- a/scripts/basic/bin2c.c
+++ b/scripts/basic/bin2c.c
@@ -29,7 +29,8 @@ int main(int argc, char *argv[])
} while (ch != EOF);

if (argc > 1)
-   printf("\t;\n\nconst int %s_size = %d;\n", argv[1], total);
+   printf("\t;\n\n#include \n\nconst size_t %s_size 
= %d;\n",
+  argv[1], total);

return 0;
 }
--
2.7.3.AMZN

[PATCH] scripts: Fix size mismatch of kexec_purgatory_size

[Tautschnig, Michael]

bin2c is used to create a valid C file out of a binary file where two
symbols will be globally defined:  and _size.  is
passed as the first parameter of the host binary.

Building using goto-cc reported that the purgatory binary code (the only
current user of this utility) declares kexec_purgatory_size as 'size_t'
where bin2c generate _size to be 'int' so in a 64-bit host where
sizeof(size_t) > sizeof(int) this type mismatch will always yield the
wrong value for big-endian architectures while for little-endian it will
be wrong if the object laid in memory directly after
kexec_purgatory_size contains non-zero value at the time of reading.

This commit changes _size to be size_t instead.

Note:

Another way to fix the problem is to change the type of
kexec_purgatory_size to be 'int' as there's this check in code:
(kexec_purgatory_size <= 0)

Signed-off-by: Michael Tautschnig 
Cc: Michal Marek 
Cc: Vivek Goyal 
---
 scripts/basic/bin2c.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/basic/bin2c.c b/scripts/basic/bin2c.c
index af187e6..c3d7eef 100644
--- a/scripts/basic/bin2c.c
+++ b/scripts/basic/bin2c.c
@@ -29,7 +29,8 @@ int main(int argc, char *argv[])
} while (ch != EOF);

if (argc > 1)
-   printf("\t;\n\nconst int %s_size = %d;\n", argv[1], total);
+   printf("\t;\n\n#include \n\nconst size_t %s_size 
= %d;\n",
+  argv[1], total);

return 0;
 }
--
2.7.3.AMZN

[PATCH] Syscall arguments are unsigned long (full registers)

[Tautschnig, Michael]

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 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: Jaswinder Singh 
Cc: Andi Kleen 
---
 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 = >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

[PATCH] Syscall arguments are unsigned long (full registers)

[Tautschnig, Michael]

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 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: Jaswinder Singh 
Cc: Andi Kleen 
---
 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 = >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