Hi Greg, Al,
The patch that went into mainline and was cc'ed to stable assumes that 2cf09666 "make SYSCALL_DEFINE<n>-generated wrappers do asmlinkage_protect... and switch i386 to HAVE_SYSCALL_WRAPPERS, killing open-coded uses of asmlinkage_protect() in a bunch of syscalls." is also included. I think my original patch would be better suited for the stable tree? It's tiny, independent, and it fixes the issue. See attached patch... Al, what do you think?
Also: please note that the problem was introduced in v3.9, so this is the only stable tree that should get this patch.
Greetings,
Alexander
On Fri, May 10, 2013, at 23:00, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
>
> This is a note to let you know that I've just added the patch titled
>
> x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)
>
> to the 3.9-stable tree which can be found at:
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
> x86-vm86-fix-vm86-syscalls-use-syscall_definex.patch
> and it can be found in the queue-3.9 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@xxxxxxxxxxxxxxx> know about it.
>
>
> From 5522ddb3fc0dfd4a503c8278eafd88c9f2d3fada Mon Sep 17 00:00:00 2001
> From: Alexander van Heukelum <heukelum@xxxxxxxxxxx>
> Date: Wed, 27 Mar 2013 22:18:05 +0100
> Subject: x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)
>
> From: Alexander van Heukelum <heukelum@xxxxxxxxxxx>
>
> commit 5522ddb3fc0dfd4a503c8278eafd88c9f2d3fada upstream.
>
> 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.
>
> [AV: killed asmlinkage_protect() - it's done automatically now]
>
> Reported-and-tested-by: Hans de Bruin <jmdebruin@xxxxxxxxx>
> Signed-off-by: Alexander van Heukelum <heukelum@xxxxxxxxxxx>
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>
> ---
> arch/x86/include/asm/syscalls.h | 4 ++--
> arch/x86/kernel/vm86_32.c | 38 ++++++++++++++------------------------
> 2 files changed, 16 insertions(+), 26 deletions(-)
>
> --- a/arch/x86/include/asm/syscalls.h
> +++ b/arch/x86/include/asm/syscalls.h
> @@ -37,8 +37,8 @@ asmlinkage int sys_get_thread_area(struc
> 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 */
>
> --- 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,36 +202,32 @@ 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.
> * This remains on the stack until we
> * return to 32 bit user space.
> */
> - struct task_struct *tsk;
> - int tmp, ret = -EPERM;
> + struct task_struct *tsk = current;
> + int tmp;
>
> - tsk = current;
> if (tsk->thread.saved_sp0)
> - goto out;
> + return -EPERM;
> tmp = copy_vm86_regs_from_user(&info.regs, &v86->regs,
> offsetof(struct kernel_vm86_struct, vm86plus) -
> sizeof(info.regs));
> - ret = -EFAULT;
> if (tmp)
> - goto out;
> + return -EFAULT;
> memset(&info.vm86plus, 0, (int)&info.regs32 - (int)&info.vm86plus);
> info.regs32 = current_pt_regs();
> tsk->thread.vm86_info = v86;
> do_sys_vm86(&info, tsk);
> - ret = 0; /* we never return here */
> -out:
> - return ret;
> + return 0; /* we never return here */
> }
>
>
> -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.
> @@ -239,7 +235,7 @@ int sys_vm86(unsigned long cmd, unsigned
> * return to 32 bit user space.
> */
> struct task_struct *tsk;
> - int tmp, ret;
> + int tmp;
> struct vm86plus_struct __user *v86;
>
> tsk = current;
> @@ -248,8 +244,7 @@ int sys_vm86(unsigned long cmd, unsigned
> case VM86_FREE_IRQ:
> case VM86_GET_IRQ_BITS:
> case VM86_GET_AND_RESET_IRQ:
> - ret = do_vm86_irq_handling(cmd, (int)arg);
> - goto out;
> + return do_vm86_irq_handling(cmd, (int)arg);
> case VM86_PLUS_INSTALL_CHECK:
> /*
> * NOTE: on old vm86 stuff this will return the error
> @@ -257,28 +252,23 @@ int sys_vm86(unsigned long cmd, unsigned
> * interpreted as (invalid) address to vm86_struct.
> * So the installation check works.
> */
> - ret = 0;
> - goto out;
> + return 0;
> }
>
> /* we come here only for functions VM86_ENTER, VM86_ENTER_NO_BYPASS */
> - ret = -EPERM;
> if (tsk->thread.saved_sp0)
> - goto out;
> + return -EPERM;
> v86 = (struct vm86plus_struct __user *)arg;
> tmp = copy_vm86_regs_from_user(&info.regs, &v86->regs,
> offsetof(struct kernel_vm86_struct, regs32) -
> sizeof(info.regs));
> - ret = -EFAULT;
> if (tmp)
> - goto out;
> + return -EFAULT;
> info.regs32 = current_pt_regs();
> info.vm86plus.is_vm86pus = 1;
> tsk->thread.vm86_info = (struct vm86_struct __user *)v86;
> do_sys_vm86(&info, tsk);
> - ret = 0; /* we never return here */
> -out:
> - return ret;
> + return 0; /* we never return here */
> }
>
>
>
>
> Patches currently in stable-queue which might be from heukelum@xxxxxxxxxxx are
>
> queue-3.9/x86-vm86-fix-vm86-syscalls-use-syscall_definex.patch
From 961a1b130aa79acb54f556a0accfcc643d1d3ed1 Mon Sep 17 00:00:00 2001
From: Alexander van Heukelum <heukelum@xxxxxxxxxxx>
Date: Tue, 26 Mar 2013 21:57:43 +0100
Subject: [PATCH] x86, vm86: fix VM86 syscalls: use asmlinkage calling convention
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 asmlinkage calling convention.
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@xxxxxxxxx>
Signed-off-by: Alexander van Heukelum <heukelum@xxxxxxxxxxx>
---
arch/x86/include/asm/syscalls.h | 4 ++--
arch/x86/kernel/vm86_32.c | 6 ++++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/syscalls.h b/arch/x86/include/asm/syscalls.h
index 6cf0a9c..a245b88 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 int sys_vm86old(struct vm86_struct __user *);
+asmlinkage int 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..7f72807 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -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)
+asmlinkage int sys_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)
+asmlinkage int sys_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