* [PATCH 1/2] powerpc: ptrace CHECK_FULL_REGS
@ 2007-09-24 23:50 Roland McGrath
2007-09-25 0:33 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 4+ messages in thread
From: Roland McGrath @ 2007-09-24 23:50 UTC (permalink / raw)
To: Paul Mackerras, linuxppc-dev
Cc: David Woodhouse, Andrew Morton, Linus Torvalds, linux-kernel
This restores the CHECK_FULL_REGS sanity check to every place that can
access the nonvolatile GPRs for ptrace. This is already done for
native-bitwidth PTRACE_PEEKUSR, but was omitted for many other cases
(32-bit ptrace, PTRACE_GETREGS, etc.); I think there may have been more
uniform checks before that were lost in the recent cleanup of GETREGS et al.
Signed-off-by: Roland McGrath <roland@redhat.com>
---
arch/powerpc/kernel/ptrace.c | 4 ++++
arch/powerpc/kernel/ptrace32.c | 8 ++++++++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 8a177bd..40d34b3 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -331,6 +331,7 @@ static long arch_ptrace_old(struct task_struct *child, long request, long addr,
unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
unsigned long __user *tmp = (unsigned long __user *)addr;
+ CHECK_FULL_REGS(child->thread.regs);
for (i = 0; i < 32; i++) {
ret = put_user(*reg, tmp);
if (ret)
@@ -346,6 +347,7 @@ static long arch_ptrace_old(struct task_struct *child, long request, long addr,
unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
unsigned long __user *tmp = (unsigned long __user *)addr;
+ CHECK_FULL_REGS(child->thread.regs);
for (i = 0; i < 32; i++) {
ret = get_user(*reg, tmp);
if (ret)
@@ -517,6 +519,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
ret = -EIO;
break;
}
+ CHECK_FULL_REGS(child->thread.regs);
ret = 0;
for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
ret |= __put_user(ptrace_get_reg(child, ui),
@@ -537,6 +540,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
ret = -EIO;
break;
}
+ CHECK_FULL_REGS(child->thread.regs);
ret = 0;
for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
ret = __get_user(tmp, (unsigned long __user *) data);
diff --git a/arch/powerpc/kernel/ptrace32.c b/arch/powerpc/kernel/ptrace32.c
index 9e6baea..6b86960 100644
--- a/arch/powerpc/kernel/ptrace32.c
+++ b/arch/powerpc/kernel/ptrace32.c
@@ -53,6 +53,7 @@ static long compat_ptrace_old(struct task_struct *child, long request,
unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
unsigned int __user *tmp = (unsigned int __user *)addr;
+ CHECK_FULL_REGS(child->thread.regs);
for (i = 0; i < 32; i++) {
ret = put_user(*reg, tmp);
if (ret)
@@ -68,6 +69,7 @@ static long compat_ptrace_old(struct task_struct *child, long request,
unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
unsigned int __user *tmp = (unsigned int __user *)addr;
+ CHECK_FULL_REGS(child->thread.regs);
for (i = 0; i < 32; i++) {
ret = get_user(*reg, tmp);
if (ret)
@@ -164,6 +166,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
if ((addr & 3) || (index > PT_FPSCR32))
break;
+ CHECK_FULL_REGS(child->thread.regs);
if (index < PT_FPR0) {
tmp = ptrace_get_reg(child, index);
} else {
@@ -210,6 +213,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
if ((addr & 3) || numReg > PT_FPSCR)
break;
+ CHECK_FULL_REGS(child->thread.regs);
if (numReg >= PT_FPR0) {
flush_fp_to_thread(child);
tmp = ((unsigned long int *)child->thread.fpr)[numReg - PT_FPR0];
@@ -270,6 +274,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
if ((addr & 3) || (index > PT_FPSCR32))
break;
+ CHECK_FULL_REGS(child->thread.regs);
if (index < PT_FPR0) {
ret = ptrace_put_reg(child, index, data);
} else {
@@ -307,6 +312,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
*/
if ((addr & 3) || (numReg > PT_FPSCR))
break;
+ CHECK_FULL_REGS(child->thread.regs);
if (numReg < PT_FPR0) {
unsigned long freg = ptrace_get_reg(child, numReg);
if (index % 2)
@@ -342,6 +348,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
ret = -EIO;
break;
}
+ CHECK_FULL_REGS(child->thread.regs);
ret = 0;
for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
ret |= __put_user(ptrace_get_reg(child, ui),
@@ -359,6 +366,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
ret = -EIO;
break;
}
+ CHECK_FULL_REGS(child->thread.regs);
ret = 0;
for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
ret = __get_user(tmp, (unsigned int __user *) data);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] powerpc: ptrace CHECK_FULL_REGS
2007-09-24 23:50 [PATCH 1/2] powerpc: ptrace CHECK_FULL_REGS Roland McGrath
@ 2007-09-25 0:33 ` Benjamin Herrenschmidt
2007-09-25 0:59 ` Roland McGrath
0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2007-09-25 0:33 UTC (permalink / raw)
To: Roland McGrath
Cc: David Woodhouse, linux-kernel, linuxppc-dev, Paul Mackerras,
Andrew Morton, Linus Torvalds
On Mon, 2007-09-24 at 16:50 -0700, Roland McGrath wrote:
> This restores the CHECK_FULL_REGS sanity check to every place that can
> access the nonvolatile GPRs for ptrace. This is already done for
> native-bitwidth PTRACE_PEEKUSR, but was omitted for many other cases
> (32-bit ptrace, PTRACE_GETREGS, etc.); I think there may have been more
> uniform checks before that were lost in the recent cleanup of GETREGS et al.
Yup, I think I ditched most of them.. for some reason I decided it
couldn't happen, but maybe I'm wrong ?
Cheers,
Ben.
> Signed-off-by: Roland McGrath <roland@redhat.com>
> ---
> arch/powerpc/kernel/ptrace.c | 4 ++++
> arch/powerpc/kernel/ptrace32.c | 8 ++++++++
> 2 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 8a177bd..40d34b3 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -331,6 +331,7 @@ static long arch_ptrace_old(struct task_struct *child, long request, long addr,
> unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
> unsigned long __user *tmp = (unsigned long __user *)addr;
>
> + CHECK_FULL_REGS(child->thread.regs);
> for (i = 0; i < 32; i++) {
> ret = put_user(*reg, tmp);
> if (ret)
> @@ -346,6 +347,7 @@ static long arch_ptrace_old(struct task_struct *child, long request, long addr,
> unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
> unsigned long __user *tmp = (unsigned long __user *)addr;
>
> + CHECK_FULL_REGS(child->thread.regs);
> for (i = 0; i < 32; i++) {
> ret = get_user(*reg, tmp);
> if (ret)
> @@ -517,6 +519,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
> ret = -EIO;
> break;
> }
> + CHECK_FULL_REGS(child->thread.regs);
> ret = 0;
> for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
> ret |= __put_user(ptrace_get_reg(child, ui),
> @@ -537,6 +540,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
> ret = -EIO;
> break;
> }
> + CHECK_FULL_REGS(child->thread.regs);
> ret = 0;
> for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
> ret = __get_user(tmp, (unsigned long __user *) data);
> diff --git a/arch/powerpc/kernel/ptrace32.c b/arch/powerpc/kernel/ptrace32.c
> index 9e6baea..6b86960 100644
> --- a/arch/powerpc/kernel/ptrace32.c
> +++ b/arch/powerpc/kernel/ptrace32.c
> @@ -53,6 +53,7 @@ static long compat_ptrace_old(struct task_struct *child, long request,
> unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
> unsigned int __user *tmp = (unsigned int __user *)addr;
>
> + CHECK_FULL_REGS(child->thread.regs);
> for (i = 0; i < 32; i++) {
> ret = put_user(*reg, tmp);
> if (ret)
> @@ -68,6 +69,7 @@ static long compat_ptrace_old(struct task_struct *child, long request,
> unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
> unsigned int __user *tmp = (unsigned int __user *)addr;
>
> + CHECK_FULL_REGS(child->thread.regs);
> for (i = 0; i < 32; i++) {
> ret = get_user(*reg, tmp);
> if (ret)
> @@ -164,6 +166,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> if ((addr & 3) || (index > PT_FPSCR32))
> break;
>
> + CHECK_FULL_REGS(child->thread.regs);
> if (index < PT_FPR0) {
> tmp = ptrace_get_reg(child, index);
> } else {
> @@ -210,6 +213,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> if ((addr & 3) || numReg > PT_FPSCR)
> break;
>
> + CHECK_FULL_REGS(child->thread.regs);
> if (numReg >= PT_FPR0) {
> flush_fp_to_thread(child);
> tmp = ((unsigned long int *)child->thread.fpr)[numReg - PT_FPR0];
> @@ -270,6 +274,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> if ((addr & 3) || (index > PT_FPSCR32))
> break;
>
> + CHECK_FULL_REGS(child->thread.regs);
> if (index < PT_FPR0) {
> ret = ptrace_put_reg(child, index, data);
> } else {
> @@ -307,6 +312,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> */
> if ((addr & 3) || (numReg > PT_FPSCR))
> break;
> + CHECK_FULL_REGS(child->thread.regs);
> if (numReg < PT_FPR0) {
> unsigned long freg = ptrace_get_reg(child, numReg);
> if (index % 2)
> @@ -342,6 +348,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> ret = -EIO;
> break;
> }
> + CHECK_FULL_REGS(child->thread.regs);
> ret = 0;
> for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
> ret |= __put_user(ptrace_get_reg(child, ui),
> @@ -359,6 +366,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> ret = -EIO;
> break;
> }
> + CHECK_FULL_REGS(child->thread.regs);
> ret = 0;
> for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
> ret = __get_user(tmp, (unsigned int __user *) data);
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] powerpc: ptrace CHECK_FULL_REGS
2007-09-25 0:33 ` Benjamin Herrenschmidt
@ 2007-09-25 0:59 ` Roland McGrath
2007-09-25 3:58 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 4+ messages in thread
From: Roland McGrath @ 2007-09-25 0:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: David Woodhouse, linux-kernel, linuxppc-dev, Paul Mackerras,
Andrew Morton, Linus Torvalds
> Yup, I think I ditched most of them.. for some reason I decided it
> couldn't happen, but maybe I'm wrong ?
Well, it's a BUG_ON. It's supposed to be for something that "can't happen".
That's why it's a sanity check, not a wild assertion. ;-)
The 2/2 patch is an example of a bug that CHECK_FULL_REGS catches.
In the status quo, using PTRACE_PEEKUSR in a bug case crashes while using
PTRACE_GETREGS in the same place might get bogus data. (In the actual bug
thus found, the data is not bogus and only the bit that FULL_REGS checks is.)
Thanks,
Roland
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] powerpc: ptrace CHECK_FULL_REGS
2007-09-25 0:59 ` Roland McGrath
@ 2007-09-25 3:58 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2007-09-25 3:58 UTC (permalink / raw)
To: Roland McGrath
Cc: David Woodhouse, linux-kernel, linuxppc-dev, Paul Mackerras,
Andrew Morton, Linus Torvalds
On Mon, 2007-09-24 at 17:59 -0700, Roland McGrath wrote:
> > Yup, I think I ditched most of them.. for some reason I decided it
> > couldn't happen, but maybe I'm wrong ?
>
> Well, it's a BUG_ON. It's supposed to be for something that "can't happen".
> That's why it's a sanity check, not a wild assertion. ;-)
>
> The 2/2 patch is an example of a bug that CHECK_FULL_REGS catches.
> In the status quo, using PTRACE_PEEKUSR in a bug case crashes while using
> PTRACE_GETREGS in the same place might get bogus data. (In the actual bug
> thus found, the data is not bogus and only the bit that FULL_REGS checks is.)
Fair enough.
Ben.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2007-09-25 3:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-24 23:50 [PATCH 1/2] powerpc: ptrace CHECK_FULL_REGS Roland McGrath
2007-09-25 0:33 ` Benjamin Herrenschmidt
2007-09-25 0:59 ` Roland McGrath
2007-09-25 3:58 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).