Re: [PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
* Linus Torvalds wrote: > On Wed, Aug 5, 2015 at 10:48 AM, Ingo Molnar wrote: > >> > >> Some of these field names are visible to userspace and can't change. > > > > That's a misconception: bits in the uapi headers can be renamed just fine. > > I disagree. If it causes pain for user space, we just shouldn't do it. Ok, agreed - I wanted to argue against the "can't change" statement and went overboard with my own statement ... Quite often headers can change and we've changed a number of fields in the past - but if they cause pain (as in this case) we don't do it. So I'd say that based on past experience: - changing small details in uapi headers is usually fine. - changing small details in the ABI is usually not fine. If anything breaks then the policy is the same, regardless of likelihood: reverting the change. > Some people copy the headers (preferred), others include the kernel header > directory with a include path or a symlink, and it's damn painful if we start > changing things that user space depends on. > > I'd say that it's like the ABI - if people don't really notice, you can do > it, > but if it breaks the build of a user app, we should be very very careful. The > breakage is often hard to fix because of nasty versioning issues.. > > Our goal in life really is "don't screw up user space". Yeah, agreed 100%. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
On Wed, Aug 5, 2015 at 10:48 AM, Ingo Molnar wrote: >> >> Some of these field names are visible to userspace and can't change. > > That's a misconception: bits in the uapi headers can be renamed just fine. I disagree. If it causes pain for user space, we just shouldn't do it. Some people copy the headers (preferred), others include the kernel header directory with a include path or a symlink, and it's damn painful if we start changing things that user space depends on. I'd say that it's like the ABI - if people don't really notice, you can do it, but if it breaks the build of a user app, we should be very very careful. The breakage is often hard to fix because of nasty versioning issues.. Our goal in life really is "don't screw up user space". Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
* Brian Gerst wrote: > > Btw., the variable names here are crazy. I had to look twice to realize > > that > > we have 'v86' and 'vm86' which are two different things. > > > > Also, vm86plus_struct variables and fields are named wildly inconsistently: > > sometimes it's 'vm86.vm86_info', sometimes it's 'v86', sometimes 'user'. > > Ugh. > > > > Other fields have naming inconsistencies as well: for example we have > > thread.vm86->vm86plus.vm86dbg_active. 'vm86' is repeated _three_ times in > > that > > name, for no good reason. > > > > So please clean up the naming to make this all easier to read. Only the > > highest level field should have 'vm86' in it - all subsequent fields will > > inherit that name one way or another. > > Some of these field names are visible to userspace and can't change. That's a misconception: bits in the uapi headers can be renamed just fine. The kernel ABI is that _semantics_ that user-space code relies on must not change. Cleaning up field names is absolutely legit to do, and we've done it numerous times in the past. Especially where they are so confusing and inconsistent as in the vm86 code. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
On Tue, Jul 21, 2015 at 3:11 AM, Ingo Molnar wrote:
>
> * Brian Gerst wrote:
>
>> --- a/arch/x86/kernel/process.c
>> +++ b/arch/x86/kernel/process.c
>> @@ -110,6 +110,13 @@ void exit_thread(void)
>> kfree(bp);
>> }
>>
>> +#ifdef CONFIG_VM86
>> + if (t->vm86) {
>> + kfree(t->vm86);
>> + t->vm86 = NULL;
>> + }
>> +#endif
>
> This should be a helper:
>
> vm86__free(t->vm86)
>
> or so.
>
>> diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
>> index e6c2b47..dce0a1c 100644
>> --- a/arch/x86/kernel/vm86_32.c
>> +++ b/arch/x86/kernel/vm86_32.c
>
>> @@ -242,9 +244,16 @@ static long do_sys_vm86(struct vm86plus_struct __user
>> *v86, bool plus,
>> {
>> struct tss_struct *tss;
>> struct task_struct *tsk = current;
>> + struct kernel_vm86_info *vm86 = tsk->thread.vm86;
>> unsigned long err = 0;
>>
>> - if (tsk->thread.saved_sp0)
>> + if (!vm86)
>> + {
>
> Non-standard style.
>
>> + if (!(vm86 = kzalloc(sizeof(*vm86), GFP_KERNEL)))
>> + return -ENOMEM;
>> + tsk->thread.vm86 = vm86;
>> + }
>> + if (vm86->saved_sp0)
>> return -EPERM;
>
>
> Btw., the variable names here are crazy. I had to look twice to realize that
> we
> have 'v86' and 'vm86' which are two different things.
>
> Also, vm86plus_struct variables and fields are named wildly inconsistently:
> sometimes it's 'vm86.vm86_info', sometimes it's 'v86', sometimes 'user'. Ugh.
>
> Other fields have naming inconsistencies as well: for example we have
> thread.vm86->vm86plus.vm86dbg_active. 'vm86' is repeated _three_ times in that
> name, for no good reason.
>
> So please clean up the naming to make this all easier to read. Only the
> highest
> level field should have 'vm86' in it - all subsequent fields will inherit that
> name one way or another.
Some of these field names are visible to userspace and can't change.
> At a quick glance I'd do the following renames:
>
> struct kernel_vm86_info *vm86; => struct vm86 *vm86;
>
> - it's obviously 'information' so the _info is superfluous.
>
> - and it's obviously embedded in a kernel structure, so the kernel_ is
> superfluous as well.
I think there is some merit to keeping the kernel prefix to make it
clear it is not the userspace version, which is similar,
> Then let's look at other fields of the main structure:
>
> struct kernel_vm86_info {
> struct vm86plus_struct __user *vm86_info;
> struct pt_regs regs32;
> unsigned long v86flags;
> unsigned long v86mask;
> unsigned long saved_sp0;
>
> unsigned long flags;
> unsigned long screen_bitmap;
> unsigned long cpu_type;
> struct revectored_struct int_revectored;
> struct revectored_struct int21_revectored;
> struct vm86plus_info_struct vm86plus;
> };
>
> - Why is there a vm86.flags and a vm86.v86flags field? What's the difference
>and can we eliminate the confusion factor?
The first is a feature flag (only used for screen bitmap), and the
latter is a shadow copy of EFLAGS.
> - The fields flags..vm86plus seems to be an as-is copy of 'struct
>vm86plus_struct'. Could this be organized in a smarter fashion?.
I specifically want to exclude retaining a copy of vm86_regs in the
kernel data. It's not needed after the syscall returns to vm86 mode.
I had originally copied all the fields after regs as a whole, but Andy
objected to that. So I changed it to copy all the fields individually
instead, so that the order of kernel vs. userspace doesn't matter.
> - 'struct vm86_regs' appears to be an as-is copy of 32-bit pt_regs, plus:
>
> unsigned short es, __esh;
> unsigned short ds, __dsh;
> unsigned short fs, __fsh;
> unsigned short gs, __gsh;
>
> Instead of a slightly different structure copying pt_regs, why not
> express it
> as:
>
> struct vm86_regs {
> struct pt_regs regs;
>
> unsigned short es, __esh;
> unsigned short ds, __dsh;
> unsigned short fs, __fsh;
> unsigned short gs, __gsh;
> };
This would be a userspace visible change. The register order and
field names from userspace are fixed, and I don't want it to depend on
the order of fields of the in-kernel pt_regs. It was a bad ABI design
choice, but it's too late to fix it now.
> - There's a number of 'long' fields which are always 32-bit, which is pretty
>confusing even if it's only ever built on 32-bit kerenls, can we use
>u8/u16/u32/u64 for ABI components instead?
Will do.
>
> Thanks,
>
> Ingo
--
Brian Gerst
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
* Brian Gerst wrote:
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -110,6 +110,13 @@ void exit_thread(void)
> kfree(bp);
> }
>
> +#ifdef CONFIG_VM86
> + if (t->vm86) {
> + kfree(t->vm86);
> + t->vm86 = NULL;
> + }
> +#endif
This should be a helper:
vm86__free(t->vm86)
or so.
> diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
> index e6c2b47..dce0a1c 100644
> --- a/arch/x86/kernel/vm86_32.c
> +++ b/arch/x86/kernel/vm86_32.c
> @@ -242,9 +244,16 @@ static long do_sys_vm86(struct vm86plus_struct __user
> *v86, bool plus,
> {
> struct tss_struct *tss;
> struct task_struct *tsk = current;
> + struct kernel_vm86_info *vm86 = tsk->thread.vm86;
> unsigned long err = 0;
>
> - if (tsk->thread.saved_sp0)
> + if (!vm86)
> + {
Non-standard style.
> + if (!(vm86 = kzalloc(sizeof(*vm86), GFP_KERNEL)))
> + return -ENOMEM;
> + tsk->thread.vm86 = vm86;
> + }
> + if (vm86->saved_sp0)
> return -EPERM;
Btw., the variable names here are crazy. I had to look twice to realize that we
have 'v86' and 'vm86' which are two different things.
Also, vm86plus_struct variables and fields are named wildly inconsistently:
sometimes it's 'vm86.vm86_info', sometimes it's 'v86', sometimes 'user'. Ugh.
Other fields have naming inconsistencies as well: for example we have
thread.vm86->vm86plus.vm86dbg_active. 'vm86' is repeated _three_ times in that
name, for no good reason.
So please clean up the naming to make this all easier to read. Only the highest
level field should have 'vm86' in it - all subsequent fields will inherit that
name one way or another.
At a quick glance I'd do the following renames:
struct kernel_vm86_info *vm86; => struct vm86 *vm86;
- it's obviously 'information' so the _info is superfluous.
- and it's obviously embedded in a kernel structure, so the kernel_ is
superfluous as well.
Then let's look at other fields of the main structure:
struct kernel_vm86_info {
struct vm86plus_struct __user *vm86_info;
struct pt_regs regs32;
unsigned long v86flags;
unsigned long v86mask;
unsigned long saved_sp0;
unsigned long flags;
unsigned long screen_bitmap;
unsigned long cpu_type;
struct revectored_struct int_revectored;
struct revectored_struct int21_revectored;
struct vm86plus_info_struct vm86plus;
};
- Why is there a vm86.flags and a vm86.v86flags field? What's the difference
and can we eliminate the confusion factor?
- The fields flags..vm86plus seems to be an as-is copy of 'struct
vm86plus_struct'. Could this be organized in a smarter fashion?.
- 'struct vm86_regs' appears to be an as-is copy of 32-bit pt_regs, plus:
unsigned short es, __esh;
unsigned short ds, __dsh;
unsigned short fs, __fsh;
unsigned short gs, __gsh;
Instead of a slightly different structure copying pt_regs, why not express
it
as:
struct vm86_regs {
struct pt_regs regs;
unsigned short es, __esh;
unsigned short ds, __dsh;
unsigned short fs, __fsh;
unsigned short gs, __gsh;
};
?
- There's a number of 'long' fields which are always 32-bit, which is pretty
confusing even if it's only ever built on 32-bit kerenls, can we use
u8/u16/u32/u64 for ABI components instead?
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
* Andy Lutomirski wrote: > On Mon, Jul 20, 2015 at 11:28 PM, Ingo Molnar wrote: > > > > * Brian Gerst wrote: > > > >> Allocate a separate structure for the vm86 fields. > > > > Why is this allocated dynamically? This structure is not very large, and a > > hole in thread_struct isn't that big of an issue - compared to additional > > fragility introduced by the (mostly untested by normal apps) dynamic > > allocation here ... > > > > I don't mind the introduction of the sub-structure in itself, but please > > embedd it in thread_struct. > > This ends up being several hundred bytes, I think, due to including an entire > struct pt_regs. Do we really want to enlarge thread_struct that much for > something that's essentially never used? Ok, I only judged by the first patch, I did not realize it becomes that much larger in later patches. I've extended the changelog to explain this properly. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
* Ingo Molnar wrote: > * Brian Gerst wrote: > > > Allocate a separate structure for the vm86 fields. > > Why is this allocated dynamically? This structure is not very large, and a > hole > in thread_struct isn't that big of an issue - compared to additional > fragility > introduced by the (mostly untested by normal apps) dynamic allocation here ... > > I don't mind the introduction of the sub-structure in itself, but please > embedd > it in thread_struct. Otherwise I have no objections to the rest of the series, nice fixes! Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
On Mon, Jul 20, 2015 at 11:28 PM, Ingo Molnar wrote: > > * Brian Gerst wrote: > >> Allocate a separate structure for the vm86 fields. > > Why is this allocated dynamically? This structure is not very large, and a > hole in > thread_struct isn't that big of an issue - compared to additional fragility > introduced by the (mostly untested by normal apps) dynamic allocation here ... > > I don't mind the introduction of the sub-structure in itself, but please > embedd it > in thread_struct. > This ends up being several hundred bytes, I think, due to including an entire struct pt_regs. Do we really want to enlarge thread_struct that much for something that's essentially never used? --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
* Brian Gerst wrote: > Allocate a separate structure for the vm86 fields. Why is this allocated dynamically? This structure is not very large, and a hole in thread_struct isn't that big of an issue - compared to additional fragility introduced by the (mostly untested by normal apps) dynamic allocation here ... I don't mind the introduction of the sub-structure in itself, but please embedd it in thread_struct. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
Allocate a separate structure for the vm86 fields.
Signed-off-by: Brian Gerst
---
arch/x86/include/asm/processor.h | 9 ++--
arch/x86/include/asm/vm86.h | 8 +++
arch/x86/kernel/process.c| 7 ++
arch/x86/kernel/vm86_32.c| 47
arch/x86/mm/fault.c | 4 ++--
5 files changed, 47 insertions(+), 28 deletions(-)
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 35ad554..d636fb8 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -403,13 +403,9 @@ struct thread_struct {
unsigned long cr2;
unsigned long trap_nr;
unsigned long error_code;
-#ifdef CONFIG_X86_32
+#ifdef CONFIG_VM86
/* Virtual 86 mode info */
- struct vm86plus_struct __user *vm86_info;
- unsigned long screen_bitmap;
- unsigned long v86flags;
- unsigned long v86mask;
- unsigned long saved_sp0;
+ struct kernel_vm86_info *vm86;
#endif
/* IO permissions: */
unsigned long *io_bitmap_ptr;
@@ -714,7 +710,6 @@ static inline void spin_lock_prefetch(const void *x)
#define INIT_THREAD { \
.sp0= TOP_OF_INIT_STACK, \
- .vm86_info = NULL, \
.sysenter_cs= __KERNEL_CS,\
.io_bitmap_ptr = NULL, \
}
diff --git a/arch/x86/include/asm/vm86.h b/arch/x86/include/asm/vm86.h
index 1d8de3f..a02967d 100644
--- a/arch/x86/include/asm/vm86.h
+++ b/arch/x86/include/asm/vm86.h
@@ -58,6 +58,14 @@ struct kernel_vm86_struct {
*/
};
+struct kernel_vm86_info {
+ struct vm86plus_struct __user *vm86_info;
+ unsigned long screen_bitmap;
+ unsigned long v86flags;
+ unsigned long v86mask;
+ unsigned long saved_sp0;
+};
+
#ifdef CONFIG_VM86
void handle_vm86_fault(struct kernel_vm86_regs *, long);
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 9cad694..5dcd037 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -110,6 +110,13 @@ void exit_thread(void)
kfree(bp);
}
+#ifdef CONFIG_VM86
+ if (t->vm86) {
+ kfree(t->vm86);
+ t->vm86 = NULL;
+ }
+#endif
+
fpu__drop(fpu);
}
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index e6c2b47..dce0a1c 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -44,6 +44,7 @@
#include
#include
#include
+#include
#include
#include
@@ -81,8 +82,8 @@
/*
* virtual flags (16 and 32-bit versions)
*/
-#define VFLAGS (*(unsigned short *)&(current->thread.v86flags))
-#define VEFLAGS(current->thread.v86flags)
+#define VFLAGS (*(unsigned short *)&(current->thread.vm86->v86flags))
+#define VEFLAGS(current->thread.vm86->v86flags)
#define set_flags(X, new, mask) \
((X) = ((X) & ~(mask)) | ((new) & (mask)))
@@ -96,6 +97,7 @@ struct pt_regs *save_v86_state(struct kernel_vm86_regs *regs)
struct pt_regs *ret;
struct task_struct *tsk = current;
struct vm86plus_struct __user *user;
+ struct kernel_vm86_info *vm86 = current->thread.vm86;
long err = 0;
/*
@@ -105,12 +107,12 @@ struct pt_regs *save_v86_state(struct kernel_vm86_regs
*regs)
*/
local_irq_enable();
- if (!tsk->thread.vm86_info) {
+ if (!vm86 || !vm86->vm86_info) {
pr_alert("no vm86_info: BAD\n");
do_exit(SIGSEGV);
}
- set_flags(regs->pt.flags, VEFLAGS, X86_EFLAGS_VIF |
tsk->thread.v86mask);
- user = tsk->thread.vm86_info;
+ set_flags(regs->pt.flags, VEFLAGS, X86_EFLAGS_VIF | vm86->v86mask);
+ user = vm86->vm86_info;
if (!access_ok(VERIFY_WRITE, user, VMPI.is_vm86pus ?
sizeof(struct vm86plus_struct) :
@@ -137,7 +139,7 @@ struct pt_regs *save_v86_state(struct kernel_vm86_regs
*regs)
put_user_ex(regs->fs, &user->regs.fs);
put_user_ex(regs->gs, &user->regs.gs);
- put_user_ex(tsk->thread.screen_bitmap, &user->screen_bitmap);
+ put_user_ex(vm86->screen_bitmap, &user->screen_bitmap);
} put_user_catch(err);
if (err) {
pr_alert("could not access userspace vm86_info\n");
@@ -145,10 +147,10 @@ struct pt_regs *save_v86_state(struct kernel_vm86_regs
*regs)
}
tss = &per_cpu(cpu_tss, get_cpu());
- tsk->thread.sp0 = tsk->thread.saved_sp0;
+ tsk->thread.sp0 = vm86->saved_sp0;
tsk->thread.sysenter_cs = __KERNEL_CS;
load_sp0(tss, &tsk->thread);
- tsk->thread.saved_sp0 = 0;
+ vm86->saved_sp0 = 0;
p
Re: [PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
On Thu, Jul 16, 2015 at 4:46 AM, Brian Gerst wrote:
> Allocate a separate structure for the vm86 fields.
>
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -315,12 +315,12 @@ check_v8086_mode(struct pt_regs *regs, unsigned long
> address,
> {
> unsigned long bit;
>
> - if (!v8086_mode(regs))
> + if (!v8086_mode(regs) || !tsk->thread.vm86)
> return;
>
What's this for? Shouldn't that "if (!v8086_mode(regs) ||
WARN_ON(!tsk->thread.vm86))"?
--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/7] x86/vm86: Move vm86 fields out of thread_struct
Allocate a separate structure for the vm86 fields.
Signed-off-by: Brian Gerst
---
arch/x86/include/asm/processor.h | 12 ++
arch/x86/include/asm/vm86.h | 11 +
arch/x86/kernel/process.c| 7 ++
arch/x86/kernel/vm86_32.c| 51 +++-
arch/x86/mm/fault.c | 4 ++--
5 files changed, 52 insertions(+), 33 deletions(-)
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 98190a6..d636fb8 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -403,16 +403,9 @@ struct thread_struct {
unsigned long cr2;
unsigned long trap_nr;
unsigned long error_code;
-#ifdef CONFIG_X86_32
+#ifdef CONFIG_VM86
/* Virtual 86 mode info */
- struct vm86plus_struct __user *vm86_info;
- unsigned long screen_bitmap;
- unsigned long v86flags;
- unsigned long v86mask;
- unsigned long saved_sp0;
-#ifdef CONFIG_X86_32_LAZY_GS
- unsigned intsaved_gs;
-#endif
+ struct kernel_vm86_info *vm86;
#endif
/* IO permissions: */
unsigned long *io_bitmap_ptr;
@@ -717,7 +710,6 @@ static inline void spin_lock_prefetch(const void *x)
#define INIT_THREAD { \
.sp0= TOP_OF_INIT_STACK, \
- .vm86_info = NULL, \
.sysenter_cs= __KERNEL_CS,\
.io_bitmap_ptr = NULL, \
}
diff --git a/arch/x86/include/asm/vm86.h b/arch/x86/include/asm/vm86.h
index 1d8de3f..07de263 100644
--- a/arch/x86/include/asm/vm86.h
+++ b/arch/x86/include/asm/vm86.h
@@ -58,6 +58,17 @@ struct kernel_vm86_struct {
*/
};
+struct kernel_vm86_info {
+ struct vm86plus_struct __user *vm86_info;
+ unsigned long screen_bitmap;
+ unsigned long v86flags;
+ unsigned long v86mask;
+ unsigned long saved_sp0;
+#ifdef CONFIG_X86_32_LAZY_GS
+ unsigned int saved_gs;
+#endif
+};
+
#ifdef CONFIG_VM86
void handle_vm86_fault(struct kernel_vm86_regs *, long);
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 9cad694..5dcd037 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -110,6 +110,13 @@ void exit_thread(void)
kfree(bp);
}
+#ifdef CONFIG_VM86
+ if (t->vm86) {
+ kfree(t->vm86);
+ t->vm86 = NULL;
+ }
+#endif
+
fpu__drop(fpu);
}
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 08bbb69..07dcab1d 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -44,6 +44,7 @@
#include
#include
#include
+#include
#include
#include
@@ -81,8 +82,8 @@
/*
* virtual flags (16 and 32-bit versions)
*/
-#define VFLAGS (*(unsigned short *)&(current->thread.v86flags))
-#define VEFLAGS(current->thread.v86flags)
+#define VFLAGS (*(unsigned short *)&(current->thread.vm86->v86flags))
+#define VEFLAGS(current->thread.vm86->v86flags)
#define set_flags(X, new, mask) \
((X) = ((X) & ~(mask)) | ((new) & (mask)))
@@ -96,6 +97,7 @@ struct pt_regs *save_v86_state(struct kernel_vm86_regs *regs)
struct pt_regs *ret;
struct task_struct *tsk = current;
struct vm86plus_struct __user *user;
+ struct kernel_vm86_info *vm86 = current->thread.vm86;
long err = 0;
/*
@@ -105,12 +107,12 @@ struct pt_regs *save_v86_state(struct kernel_vm86_regs
*regs)
*/
local_irq_enable();
- if (!tsk->thread.vm86_info) {
+ if (!vm86 || !vm86->vm86_info) {
pr_alert("no vm86_info: BAD\n");
do_exit(SIGSEGV);
}
- set_flags(regs->pt.flags, VEFLAGS, X86_EFLAGS_VIF |
tsk->thread.v86mask);
- user = tsk->thread.vm86_info;
+ set_flags(regs->pt.flags, VEFLAGS, X86_EFLAGS_VIF | vm86->v86mask);
+ user = vm86->vm86_info;
put_user_try {
put_user_ex(regs->pt.bx, &user->regs.ebx);
put_user_ex(regs->pt.cx, &user->regs.ecx);
@@ -129,7 +131,7 @@ struct pt_regs *save_v86_state(struct kernel_vm86_regs
*regs)
put_user_ex(regs->fs, &user->regs.fs);
put_user_ex(regs->gs, &user->regs.gs);
- put_user_ex(tsk->thread.screen_bitmap, &user->screen_bitmap);
+ put_user_ex(vm86->screen_bitmap, &user->screen_bitmap);
} put_user_catch(err);
if (err) {
pr_alert("could not access userspace vm86_info\n");
@@ -137,15 +139,15 @@ struct pt_regs *save_v86_state(struct kernel_vm86_regs
*regs)
}
tss = &per_cpu(cpu_tss, get_cpu());
- tsk->thread.sp0 = tsk->thread.saved_sp0;
+ tsk->thread.sp0
