Re: [PATCH v2] x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)

From: Alexander van Heukelum <heukelum@fastmail.fm>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Hans de Bruin <jmdebruin@xmsnet.nl>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH v2] x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)
Date: Mon, 15 Apr 2013 10:39:25 +0200	[thread overview]
Message-ID: <1366015165.4521.140661217861630.48211872@webmail.messagingengine.com> (raw)
In-Reply-To: <51686B74.2040204@xmsnet.nl>

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

On Fri, Apr 12, 2013, at 22:15, Hans de Bruin wrote:
> On 03/27/2013 10:18 PM, Alexander van Heukelum wrote:
> > On Wed, Mar 27, 2013, at 20:46, Al Viro wrote:
> >> On Wed, Mar 27, 2013 at 08:31:02PM +0100, Alexander van Heukelum wrote:
> >>> Hi Al,
> >>>
> >>> Hans de Bruin found a regression due to one of your changes. I asked him to test a fix and he reported back that it worked. (Thanks!) Can you see if you agree with the fix? Patch is attached due to webmail...
> >>
> >> Use SYSCALL_DEFINE{1,2} for vm86_old and vm86, please.
> >
> > Like this?

Hi Al,

Could you comment on the patch for the problem with the VM86 calls? I changed it to use SYSCALL_DEFINEx at the definition sites, and I changed the declarations in asm/syscalls.h to fit. If things are ok, can you take care of sending it to Linus?

Greetings,
    Alexander

> > Greetings,
> >      Alexander
> >
> 
> Is there any progress?
>
> -- 
> Hans

[-- Attachment #2: 0001-x86-vm86-fix-VM86-syscalls-use-SYSCALL_DEFINEx.patch --]
[-- Type: application/octet-stream, Size: 3315 bytes --]

From 450d86e6ad0a7d387cf706714c1fc030bb4b13a5 Mon Sep 17 00:00:00 2001
From: Alexander van Heukelum <heukelum@fastmail.fm>
Date: Tue, 26 Mar 2013 21:57:43 +0100
Subject: [PATCH] x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)

Commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
got rid of the pt_regs stub for sys_vm86old and sys_vm86. The functions
were, however, not changed to use the calling convention for syscalls.

[v2] Use SYSCALL_DEFINEx(...). Compiles to identical code.

The regression was reported and pinpointed by Hans de Bruin:
> commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
> somehow breaks the colors when I play 'civilization I' under xdosemu.
> During the intro of the game something the colors get messed up. When
> the game begins the grass of the earth is red. Reverting the commit
> fixes the problem.

And he tested the patch too:
> Yep, the grass is green again.

Reported-and-tested-by: Hans de Bruin <jmdebruin@xmsnet.nl>
Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
 arch/x86/include/asm/syscalls.h | 4 ++--
 arch/x86/kernel/vm86_32.c       | 8 +++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/syscalls.h b/arch/x86/include/asm/syscalls.h
index 6cf0a9c..5a0be0a 100644
--- a/arch/x86/include/asm/syscalls.h
+++ b/arch/x86/include/asm/syscalls.h
@@ -37,8 +37,8 @@ asmlinkage int sys_get_thread_area(struct user_desc __user *);
 unsigned long sys_sigreturn(void);
 
 /* kernel/vm86_32.c */
-int sys_vm86old(struct vm86_struct __user *);
-int sys_vm86(unsigned long, unsigned long);
+asmlinkage long sys_vm86old(struct vm86_struct __user *);
+asmlinkage long sys_vm86(unsigned long, unsigned long);
 
 #else /* CONFIG_X86_32 */
 
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 1cf5766..a67cb2b 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -33,6 +33,7 @@
 #include <linux/capability.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
+#include <linux/syscalls.h>
 #include <linux/sched.h>
 #include <linux/kernel.h>
 #include <linux/signal.h>
@@ -48,7 +49,6 @@
 #include <asm/io.h>
 #include <asm/tlbflush.h>
 #include <asm/irq.h>
-#include <asm/syscalls.h>
 
 /*
  * Known problems:
@@ -202,7 +202,7 @@ out:
 static int do_vm86_irq_handling(int subfunction, int irqnumber);
 static void do_sys_vm86(struct kernel_vm86_struct *info, struct task_struct *tsk);
 
-int sys_vm86old(struct vm86_struct __user *v86)
+SYSCALL_DEFINE1(vm86old, struct vm86_struct __user *, v86)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 					 * this avoids wasting of stack space.
@@ -227,11 +227,12 @@ int sys_vm86old(struct vm86_struct __user *v86)
 	do_sys_vm86(&info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(1, ret, v86);
 	return ret;
 }
 
 
-int sys_vm86(unsigned long cmd, unsigned long arg)
+SYSCALL_DEFINE2(vm86, unsigned long, cmd, unsigned long, arg)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 					 * this avoids wasting of stack space.
@@ -278,6 +279,7 @@ int sys_vm86(unsigned long cmd, unsigned long arg)
 	do_sys_vm86(&info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(2, ret, cmd, arg);
 	return ret;
 }
 
-- 
1.8.1.2

