2010-02-09 20:28:27

by Suresh Siddha

[permalink] [raw]
Subject: [patch v2 3/4] x86, ptrace: prepare regset get/set routines for user specified lengths

Prepare the x86 regset routines for the upcoming generic
PTRACE_GETREGSET/PTRACE_SETREGSET commands. These commands allow the user
to specify how much to read and hence the kernel needs to ensure that
the get/set routines of the regset don't allow the user to access more kernel
buffers than needed.

Signed-off-by: Suresh Siddha <[email protected]>
---
arch/x86/kernel/i387.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

Index: tip/arch/x86/kernel/i387.c
===================================================================
--- tip.orig/arch/x86/kernel/i387.c
+++ tip/arch/x86/kernel/i387.c
@@ -184,6 +184,7 @@ int xfpregs_get(struct task_struct *targ
void *kbuf, void __user *ubuf)
{
int ret;
+ int size = regset->n * regset->size;

if (!cpu_has_fxsr)
return -ENODEV;
@@ -193,7 +194,7 @@ int xfpregs_get(struct task_struct *targ
return ret;

return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
- &target->thread.xstate->fxsave, 0, -1);
+ &target->thread.xstate->fxsave, 0, size);
}

int xfpregs_set(struct task_struct *target, const struct user_regset *regset,
@@ -201,6 +202,7 @@ int xfpregs_set(struct task_struct *targ
const void *kbuf, const void __user *ubuf)
{
int ret;
+ int size = regset->n * regset->size;

if (!cpu_has_fxsr)
return -ENODEV;
@@ -210,7 +212,7 @@ int xfpregs_set(struct task_struct *targ
return ret;

ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
- &target->thread.xstate->fxsave, 0, -1);
+ &target->thread.xstate->fxsave, 0, size);

/*
* mxcsr reserved bits must be masked to zero for security reasons.
@@ -452,6 +454,7 @@ int fpregs_get(struct task_struct *targe
void *kbuf, void __user *ubuf)
{
struct user_i387_ia32_struct env;
+ int size = regset->n * regset->size;
int ret;

ret = init_fpu(target);
@@ -464,7 +467,7 @@ int fpregs_get(struct task_struct *targe
if (!cpu_has_fxsr) {
return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
&target->thread.xstate->fsave, 0,
- -1);
+ size);
}

if (kbuf && pos == 0 && count == sizeof(env)) {
@@ -482,6 +485,7 @@ int fpregs_set(struct task_struct *targe
const void *kbuf, const void __user *ubuf)
{
struct user_i387_ia32_struct env;
+ int size = regset->n * regset->size;
int ret;

ret = init_fpu(target);
@@ -495,13 +499,14 @@ int fpregs_set(struct task_struct *targe

if (!cpu_has_fxsr) {
return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
- &target->thread.xstate->fsave, 0, -1);
+ &target->thread.xstate->fsave, 0,
+ size);
}

if (pos > 0 || count < sizeof(env))
convert_from_fxsr(&env, target);

- ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &env, 0, -1);
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &env, 0, size);
if (!ret)
convert_to_fxsr(target, &env);



2010-02-09 22:55:32

by Suresh Siddha

[permalink] [raw]
Subject: [tip:x86/ptrace] x86, ptrace: Prepare regset get/set routines for user specified lengths

Commit-ID: a6bf9383255e088f5a78d93e2f8cd871aa028060
Gitweb: http://git.kernel.org/tip/a6bf9383255e088f5a78d93e2f8cd871aa028060
Author: Suresh Siddha <[email protected]>
AuthorDate: Tue, 9 Feb 2010 12:13:12 -0800
Committer: H. Peter Anvin <[email protected]>
CommitDate: Tue, 9 Feb 2010 14:09:58 -0800

x86, ptrace: Prepare regset get/set routines for user specified lengths

Prepare the x86 regset routines for the upcoming generic
PTRACE_GETREGSET/PTRACE_SETREGSET commands. These commands allow the user
to specify how much to read and hence the kernel needs to ensure that
the get/set routines of the regset don't allow the user to access more kernel
buffers than needed.

Signed-off-by: Suresh Siddha <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/kernel/i387.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 4719bf9..23efdef 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -184,6 +184,7 @@ int xfpregs_get(struct task_struct *target, const struct user_regset *regset,
void *kbuf, void __user *ubuf)
{
int ret;
+ int size = regset->n * regset->size;

if (!cpu_has_fxsr)
return -ENODEV;
@@ -193,7 +194,7 @@ int xfpregs_get(struct task_struct *target, const struct user_regset *regset,
return ret;

return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
- &target->thread.xstate->fxsave, 0, -1);
+ &target->thread.xstate->fxsave, 0, size);
}

int xfpregs_set(struct task_struct *target, const struct user_regset *regset,
@@ -201,6 +202,7 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset,
const void *kbuf, const void __user *ubuf)
{
int ret;
+ int size = regset->n * regset->size;

if (!cpu_has_fxsr)
return -ENODEV;
@@ -210,7 +212,7 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset,
return ret;

ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
- &target->thread.xstate->fxsave, 0, -1);
+ &target->thread.xstate->fxsave, 0, size);

/*
* mxcsr reserved bits must be masked to zero for security reasons.
@@ -452,6 +454,7 @@ int fpregs_get(struct task_struct *target, const struct user_regset *regset,
void *kbuf, void __user *ubuf)
{
struct user_i387_ia32_struct env;
+ int size = regset->n * regset->size;
int ret;

ret = init_fpu(target);
@@ -464,7 +467,7 @@ int fpregs_get(struct task_struct *target, const struct user_regset *regset,
if (!cpu_has_fxsr) {
return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
&target->thread.xstate->fsave, 0,
- -1);
+ size);
}

if (kbuf && pos == 0 && count == sizeof(env)) {
@@ -482,6 +485,7 @@ int fpregs_set(struct task_struct *target, const struct user_regset *regset,
const void *kbuf, const void __user *ubuf)
{
struct user_i387_ia32_struct env;
+ int size = regset->n * regset->size;
int ret;

ret = init_fpu(target);
@@ -495,13 +499,14 @@ int fpregs_set(struct task_struct *target, const struct user_regset *regset,

if (!cpu_has_fxsr) {
return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
- &target->thread.xstate->fsave, 0, -1);
+ &target->thread.xstate->fsave, 0,
+ size);
}

if (pos > 0 || count < sizeof(env))
convert_from_fxsr(&env, target);

- ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &env, 0, -1);
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &env, 0, size);
if (!ret)
convert_to_fxsr(target, &env);

2010-02-10 01:32:52

by Roland McGrath

[permalink] [raw]
Subject: Re: [patch v2 3/4] x86, ptrace: prepare regset get/set routines for user specified lengths

This change is unnecessary and IMHO misguided. The arch user_regset.get
and user_regset.set functions are not required to worry about bogus
arguments. Their callers are required to pass values that don't violate
the .n, .size, and .align constraints in the user_regset. The right place
to enforce these constraints on parameters coming from userland is in the
arch-independent code that calls the arch user_regset functions.


Thanks,
Roland