* [PATCH] ptrace GET/SET FPXREGS broken
@ 2008-06-30 4:44 TAKADA Yoshihito
2008-06-30 12:18 ` Ingo Molnar
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: TAKADA Yoshihito @ 2008-06-30 4:44 UTC (permalink / raw)
To: linux-kernel
[-- Attachment #1: Type: Text/Plain, Size: 247 bytes --]
Hi
When I update kernel 2.6.25 from 2.6.24, gdb does not work.
On 2.6.25, ptrace(PTRACE_GETFPXREGS, ...) returns ENODEV.
But 2.6.24 kernel's ptrace() returns EIO.
It is issue of compatibility.
I attached test program as pt.c and patch for fix it.
[-- Attachment #2: pt.c --]
[-- Type: Text/Plain, Size: 1077 bytes --]
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <signal.h>
#include <errno.h>
#include <sys/ptrace.h>
#include <sys/types.h>
struct user_fxsr_struct {
unsigned short cwd;
unsigned short swd;
unsigned short twd;
unsigned short fop;
long fip;
long fcs;
long foo;
long fos;
long mxcsr;
long reserved;
long st_space[32]; /* 8*16 bytes for each FP-reg = 128 bytes */
long xmm_space[32]; /* 8*16 bytes for each XMM-reg = 128 bytes */
long padding[56];
};
int main(void)
{
pid_t pid;
pid = fork();
switch(pid){
case -1:/* error */
break;
case 0:/* child */
child();
break;
default:
parent(pid);
break;
}
return 0;
}
int child(void)
{
ptrace(PTRACE_TRACEME);
kill(getpid(), SIGSTOP);
sleep(10);
return 0;
}
int parent(pid_t pid)
{
int ret;
struct user_fxsr_struct fpxregs;
ret = ptrace(PTRACE_GETFPXREGS, pid, 0, &fpxregs);
if(ret < 0){
printf("%d: %s.\n", errno, strerror(errno));
}
kill(pid, SIGCONT);
wait(pid);
return 0;
}
/* in the kerel, at kernel/i387.c get_fpxregs() */
[-- Attachment #3: fxsr.patch --]
[-- Type: Text/Plain, Size: 417 bytes --]
--- a/arch/x86/kernel/i387.c 2008-04-17 11:49:44.000000000 +0900
+++ b/arch/x86/kernel/i387.c 2008-06-30 13:22:57.000000000 +0900
@@ -130,7 +130,7 @@
void *kbuf, void __user *ubuf)
{
if (!cpu_has_fxsr)
- return -ENODEV;
+ return -EIO;
init_fpu(target);
@@ -145,7 +145,7 @@
int ret;
if (!cpu_has_fxsr)
- return -ENODEV;
+ return -EIO;
init_fpu(target);
set_stopped_child_used_math(target);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ptrace GET/SET FPXREGS broken
2008-06-30 4:44 [PATCH] ptrace GET/SET FPXREGS broken TAKADA Yoshihito
@ 2008-06-30 12:18 ` Ingo Molnar
2008-06-30 21:02 ` [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error Roland McGrath
2008-06-30 21:02 ` [PATCH] ptrace GET/SET FPXREGS broken Roland McGrath
2 siblings, 0 replies; 13+ messages in thread
From: Ingo Molnar @ 2008-06-30 12:18 UTC (permalink / raw)
To: TAKADA Yoshihito; +Cc: linux-kernel, Roland McGrath, the arch/x86 maintainers
* TAKADA Yoshihito <takada@mbf.nifty.com> wrote:
> Hi
> When I update kernel 2.6.25 from 2.6.24, gdb does not work.
> On 2.6.25, ptrace(PTRACE_GETFPXREGS, ...) returns ENODEV.
> But 2.6.24 kernel's ptrace() returns EIO.
> It is issue of compatibility.
> I attached test program as pt.c and patch for fix it.
this is a side-effect of the "x86: x86 i387 user_regset" commit,
v2.6.24-4379-g4421011. Roland Cc:-ed.
Ingo
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
2008-06-30 4:44 [PATCH] ptrace GET/SET FPXREGS broken TAKADA Yoshihito
2008-06-30 12:18 ` Ingo Molnar
@ 2008-06-30 21:02 ` Roland McGrath
2008-07-01 9:02 ` Ingo Molnar
2008-07-03 1:58 ` TAKADA Yoshihito
2008-06-30 21:02 ` [PATCH] ptrace GET/SET FPXREGS broken Roland McGrath
2 siblings, 2 replies; 13+ messages in thread
From: Roland McGrath @ 2008-06-30 21:02 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner; +Cc: linux-kernel, stable, TAKADA Yoshihito
ptrace has always returned only -EIO for all failures to access
registers. The user_regset calls are allowed to return a more
meaningful variety of errors. The REGSET_XFP calls use -ENODEV
for !cpu_has_fxsr hardware. Make ptrace return the traditional
-EIO instead of the error code from the user_regset call.
Signed-off-by: Roland McGrath <roland@redhat.com>
---
arch/x86/kernel/ptrace.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index a7835f2..77040b6 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -943,13 +943,13 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
return copy_regset_to_user(child, &user_x86_32_view,
REGSET_XFP,
0, sizeof(struct user_fxsr_struct),
- datap);
+ datap) ? -EIO : 0;
case PTRACE_SETFPXREGS: /* Set the child extended FPU state. */
return copy_regset_from_user(child, &user_x86_32_view,
REGSET_XFP,
0, sizeof(struct user_fxsr_struct),
- datap);
+ datap) ? -EIO : 0;
#endif
#if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] ptrace GET/SET FPXREGS broken
2008-06-30 4:44 [PATCH] ptrace GET/SET FPXREGS broken TAKADA Yoshihito
2008-06-30 12:18 ` Ingo Molnar
2008-06-30 21:02 ` [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error Roland McGrath
@ 2008-06-30 21:02 ` Roland McGrath
2 siblings, 0 replies; 13+ messages in thread
From: Roland McGrath @ 2008-06-30 21:02 UTC (permalink / raw)
To: TAKADA Yoshihito; +Cc: linux-kernel, Ingo Molnar
Thanks for the report. That was indeed a regression from 2.6.24 in ptrace.
To match traditional behavior, ptrace should only ever return -EIO for all
kinds of errors accessing registers (except for the -ESRCH cases). But it
is cleaner for the user_regset calls to use meaningful error codes, and
-ENODEV is what makes sense for the !cpu_has_fxsr case.
So the right fix for this is in ptrace, not in user_regset.
Ingo, please revert takada's patch and apply the one about to come from me.
Thanks,
Roland
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
2008-06-30 21:02 ` [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error Roland McGrath
@ 2008-07-01 9:02 ` Ingo Molnar
2008-07-01 9:32 ` Roland McGrath
2008-07-03 2:37 ` TAKADA Yoshihito
2008-07-03 1:58 ` TAKADA Yoshihito
1 sibling, 2 replies; 13+ messages in thread
From: Ingo Molnar @ 2008-07-01 9:02 UTC (permalink / raw)
To: Roland McGrath; +Cc: Thomas Gleixner, linux-kernel, stable, TAKADA Yoshihito
* Roland McGrath <roland@redhat.com> wrote:
> ptrace has always returned only -EIO for all failures to access
> registers. The user_regset calls are allowed to return a more
> meaningful variety of errors. The REGSET_XFP calls use -ENODEV for
> !cpu_has_fxsr hardware. Make ptrace return the traditional -EIO
> instead of the error code from the user_regset call.
since the original fix is already upstream, i've applied the delta patch
below. Should we still do this for v2.6.26 or can we defer it to
v2.6.27? As ptrace is the only user of this facility for now this would
be an identity transformation AFAICS and the v2.6.26 release is very
close.
Ingo
---------------->
Subject: x86 ptrace: fix PTRACE_GETFPXREGS error
From: Roland McGrath <roland@redhat.com>
Date: Mon, 30 Jun 2008 14:02:41 -0700 (PDT)
ptrace has always returned only -EIO for all failures to access
registers. The user_regset calls are allowed to return a more
meaningful variety of errors. The REGSET_XFP calls use -ENODEV
for !cpu_has_fxsr hardware. Make ptrace return the traditional
-EIO instead of the error code from the user_regset call.
Signed-off-by: Roland McGrath <roland@redhat.com>
Cc: stable@kernel.org
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/x86/kernel/i387.c | 4 ++--
arch/x86/kernel/ptrace.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
Index: tip/arch/x86/kernel/i387.c
===================================================================
--- tip.orig/arch/x86/kernel/i387.c
+++ tip/arch/x86/kernel/i387.c
@@ -162,7 +162,7 @@ int xfpregs_get(struct task_struct *targ
int ret;
if (!cpu_has_fxsr)
- return -EIO;
+ return -ENODEV;
ret = init_fpu(target);
if (ret)
@@ -179,7 +179,7 @@ int xfpregs_set(struct task_struct *targ
int ret;
if (!cpu_has_fxsr)
- return -EIO;
+ return -ENODEV;
ret = init_fpu(target);
if (ret)
Index: tip/arch/x86/kernel/ptrace.c
===================================================================
--- tip.orig/arch/x86/kernel/ptrace.c
+++ tip/arch/x86/kernel/ptrace.c
@@ -943,13 +943,13 @@ long arch_ptrace(struct task_struct *chi
return copy_regset_to_user(child, &user_x86_32_view,
REGSET_XFP,
0, sizeof(struct user_fxsr_struct),
- datap);
+ datap) ? -EIO : 0;
case PTRACE_SETFPXREGS: /* Set the child extended FPU state. */
return copy_regset_from_user(child, &user_x86_32_view,
REGSET_XFP,
0, sizeof(struct user_fxsr_struct),
- datap);
+ datap) ? -EIO : 0;
#endif
#if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
2008-07-01 9:02 ` Ingo Molnar
@ 2008-07-01 9:32 ` Roland McGrath
2008-07-01 10:11 ` Ingo Molnar
2008-07-03 2:37 ` TAKADA Yoshihito
1 sibling, 1 reply; 13+ messages in thread
From: Roland McGrath @ 2008-07-01 9:32 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Thomas Gleixner, linux-kernel, stable, TAKADA Yoshihito
> since the original fix is already upstream, i've applied the delta patch
> below. Should we still do this for v2.6.26 or can we defer it to
> v2.6.27? As ptrace is the only user of this facility for now this would
> be an identity transformation AFAICS and the v2.6.26 release is very
> close.
I don't think there's a problem with 2.6.26 either way. I agree that the
user_regset internal API does not matter much before 2.6.27.
My patch alone applies to 2.6.25, which is why I CC'd it to stable.
I think applying that (and not takada's patch) to stable-2.6.25
would be best.
Thanks,
Roland
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
2008-07-01 9:32 ` Roland McGrath
@ 2008-07-01 10:11 ` Ingo Molnar
2008-07-01 14:34 ` [stable] " Greg KH
0 siblings, 1 reply; 13+ messages in thread
From: Ingo Molnar @ 2008-07-01 10:11 UTC (permalink / raw)
To: Roland McGrath; +Cc: Thomas Gleixner, linux-kernel, stable, TAKADA Yoshihito
* Roland McGrath <roland@redhat.com> wrote:
> > since the original fix is already upstream, i've applied the delta
> > patch below. Should we still do this for v2.6.26 or can we defer it
> > to v2.6.27? As ptrace is the only user of this facility for now this
> > would be an identity transformation AFAICS and the v2.6.26 release
> > is very close.
>
> I don't think there's a problem with 2.6.26 either way. I agree that
> the user_regset internal API does not matter much before 2.6.27.
okay - i've queued it up in tip/x86/ptrace for now.
> My patch alone applies to 2.6.25, which is why I CC'd it to stable. I
> think applying that (and not takada's patch) to stable-2.6.25 would be
> best.
i think Greg already queued the original fix up for v2.6.25, as per the
commit notifier below.
so i think it is all sorted fine now?
Ingo
---------------------->
This is a note to let you know that we have just queued up the patch titled
Subject: ptrace GET/SET FPXREGS broken
to the 2.6.25-stable tree. Its filename is
ptrace-get-set-fpxregs-broken.patch
A git repo of this tree can be found at
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>From stable-bounces@linux.kernel.org Mon Jun 30 09:22:46 2008
From: TAKADA Yoshihito <takada@mbf.nifty.com>
Date: Mon, 30 Jun 2008 18:22:07 +0200
Subject: ptrace GET/SET FPXREGS broken
To: stable@kernel.org
Message-ID: <20080630162207.GC17710@elte.hu>
Content-Disposition: inline
From: TAKADA Yoshihito <takada@mbf.nifty.com>
Commit 11dbc963a8f6128595d0f6ecf138dc369e144997 upstream
ptrace GET/SET FPXREGS broken
When I update kernel 2.6.25 from 2.6.24, gdb does not work.
On 2.6.25, ptrace(PTRACE_GETFPXREGS, ...) returns ENODEV.
But 2.6.24 kernel's ptrace() returns EIO.
It is issue of compatibility.
I attached test program as pt.c and patch for fix it.
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <signal.h>
#include <errno.h>
#include <sys/ptrace.h>
#include <sys/types.h>
struct user_fxsr_struct {
unsigned short cwd;
unsigned short swd;
unsigned short twd;
unsigned short fop;
long fip;
long fcs;
long foo;
long fos;
long mxcsr;
long reserved;
long st_space[32]; /* 8*16 bytes for each FP-reg = 128 bytes */
long xmm_space[32]; /* 8*16 bytes for each XMM-reg = 128 bytes */
long padding[56];
};
int main(void)
{
pid_t pid;
pid = fork();
switch(pid){
case -1:/* error */
break;
case 0:/* child */
child();
break;
default:
parent(pid);
break;
}
return 0;
}
int child(void)
{
ptrace(PTRACE_TRACEME);
kill(getpid(), SIGSTOP);
sleep(10);
return 0;
}
int parent(pid_t pid)
{
int ret;
struct user_fxsr_struct fpxregs;
ret = ptrace(PTRACE_GETFPXREGS, pid, 0, &fpxregs);
if(ret < 0){
printf("%d: %s.\n", errno, strerror(errno));
}
kill(pid, SIGCONT);
wait(pid);
return 0;
}
/* in the kerel, at kernel/i387.c get_fpxregs() */
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
arch/x86/kernel/i387.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -130,7 +130,7 @@ int xfpregs_get(struct task_struct *targ
void *kbuf, void __user *ubuf)
{
if (!cpu_has_fxsr)
- return -ENODEV;
+ return -EIO;
init_fpu(target);
@@ -145,7 +145,7 @@ int xfpregs_set(struct task_struct *targ
int ret;
if (!cpu_has_fxsr)
- return -ENODEV;
+ return -EIO;
init_fpu(target);
set_stopped_child_used_math(target);
Patches currently in stable-queue which might be from takada@mbf.nifty.com are
queue-2.6.25/ptrace-get-set-fpxregs-broken.patch
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [stable] [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
2008-07-01 10:11 ` Ingo Molnar
@ 2008-07-01 14:34 ` Greg KH
2008-07-01 14:46 ` Ingo Molnar
0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2008-07-01 14:34 UTC (permalink / raw)
To: Ingo Molnar
Cc: Roland McGrath, Thomas Gleixner, TAKADA Yoshihito, linux-kernel,
stable
On Tue, Jul 01, 2008 at 12:11:04PM +0200, Ingo Molnar wrote:
>
> * Roland McGrath <roland@redhat.com> wrote:
>
> > > since the original fix is already upstream, i've applied the delta
> > > patch below. Should we still do this for v2.6.26 or can we defer it
> > > to v2.6.27? As ptrace is the only user of this facility for now this
> > > would be an identity transformation AFAICS and the v2.6.26 release
> > > is very close.
> >
> > I don't think there's a problem with 2.6.26 either way. I agree that
> > the user_regset internal API does not matter much before 2.6.27.
>
> okay - i've queued it up in tip/x86/ptrace for now.
>
> > My patch alone applies to 2.6.25, which is why I CC'd it to stable. I
> > think applying that (and not takada's patch) to stable-2.6.25 would be
> > best.
>
> i think Greg already queued the original fix up for v2.6.25, as per the
> commit notifier below.
Yes I did queue that one up, but it looks different from Roland's
original patch in this thread.
Roland, does the patch below suffice? Or should I take your original
one instead, or as well?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [stable] [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
2008-07-01 14:34 ` [stable] " Greg KH
@ 2008-07-01 14:46 ` Ingo Molnar
2008-07-01 15:02 ` Greg KH
0 siblings, 1 reply; 13+ messages in thread
From: Ingo Molnar @ 2008-07-01 14:46 UTC (permalink / raw)
To: Greg KH
Cc: Roland McGrath, Thomas Gleixner, TAKADA Yoshihito, linux-kernel,
stable
* Greg KH <greg@kroah.com> wrote:
> On Tue, Jul 01, 2008 at 12:11:04PM +0200, Ingo Molnar wrote:
> >
> > * Roland McGrath <roland@redhat.com> wrote:
> >
> > > > since the original fix is already upstream, i've applied the delta
> > > > patch below. Should we still do this for v2.6.26 or can we defer it
> > > > to v2.6.27? As ptrace is the only user of this facility for now this
> > > > would be an identity transformation AFAICS and the v2.6.26 release
> > > > is very close.
> > >
> > > I don't think there's a problem with 2.6.26 either way. I agree that
> > > the user_regset internal API does not matter much before 2.6.27.
> >
> > okay - i've queued it up in tip/x86/ptrace for now.
> >
> > > My patch alone applies to 2.6.25, which is why I CC'd it to stable. I
> > > think applying that (and not takada's patch) to stable-2.6.25 would be
> > > best.
> >
> > i think Greg already queued the original fix up for v2.6.25, as per the
> > commit notifier below.
>
> Yes I did queue that one up, but it looks different from Roland's
> original patch in this thread.
yes, but as discussed in this thread, both patches are fine in principle
as far as the regression goes.
Ingo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [stable] [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
2008-07-01 14:46 ` Ingo Molnar
@ 2008-07-01 15:02 ` Greg KH
0 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2008-07-01 15:02 UTC (permalink / raw)
To: Ingo Molnar
Cc: Roland McGrath, Thomas Gleixner, TAKADA Yoshihito, linux-kernel,
stable
On Tue, Jul 01, 2008 at 04:46:58PM +0200, Ingo Molnar wrote:
>
> * Greg KH <greg@kroah.com> wrote:
>
> > On Tue, Jul 01, 2008 at 12:11:04PM +0200, Ingo Molnar wrote:
> > >
> > > * Roland McGrath <roland@redhat.com> wrote:
> > >
> > > > > since the original fix is already upstream, i've applied the delta
> > > > > patch below. Should we still do this for v2.6.26 or can we defer it
> > > > > to v2.6.27? As ptrace is the only user of this facility for now this
> > > > > would be an identity transformation AFAICS and the v2.6.26 release
> > > > > is very close.
> > > >
> > > > I don't think there's a problem with 2.6.26 either way. I agree that
> > > > the user_regset internal API does not matter much before 2.6.27.
> > >
> > > okay - i've queued it up in tip/x86/ptrace for now.
> > >
> > > > My patch alone applies to 2.6.25, which is why I CC'd it to stable. I
> > > > think applying that (and not takada's patch) to stable-2.6.25 would be
> > > > best.
> > >
> > > i think Greg already queued the original fix up for v2.6.25, as per the
> > > commit notifier below.
> >
> > Yes I did queue that one up, but it looks different from Roland's
> > original patch in this thread.
>
> yes, but as discussed in this thread, both patches are fine in principle
> as far as the regression goes.
Ah, great, sorry I missed that. I'll leave the current patch in our
tree and go get more coffee :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
2008-06-30 21:02 ` [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error Roland McGrath
2008-07-01 9:02 ` Ingo Molnar
@ 2008-07-03 1:58 ` TAKADA Yoshihito
2008-07-03 3:00 ` Roland McGrath
1 sibling, 1 reply; 13+ messages in thread
From: TAKADA Yoshihito @ 2008-07-03 1:58 UTC (permalink / raw)
To: roland; +Cc: mingo, tglx, linux-kernel, stable
Hi. Thanks for your right patch.
BTW, are FXSAVE and FXRSTOR instructions device?
Is it right to return ENODEV?
I think I had bettor return EXIO or ENOTSUP.
If it discussed, tell me URL of tree at lkml.org.
From: Roland McGrath <roland@redhat.com>
Subject: [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
Date: Mon, 30 Jun 2008 14:02:41 -0700 (PDT)
> ptrace has always returned only -EIO for all failures to access
> registers. The user_regset calls are allowed to return a more
> meaningful variety of errors. The REGSET_XFP calls use -ENODEV
> for !cpu_has_fxsr hardware. Make ptrace return the traditional
> -EIO instead of the error code from the user_regset call.
>
> Signed-off-by: Roland McGrath <roland@redhat.com>
> ---
> arch/x86/kernel/ptrace.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
> index a7835f2..77040b6 100644
> --- a/arch/x86/kernel/ptrace.c
> +++ b/arch/x86/kernel/ptrace.c
> @@ -943,13 +943,13 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
> return copy_regset_to_user(child, &user_x86_32_view,
> REGSET_XFP,
> 0, sizeof(struct user_fxsr_struct),
> - datap);
> + datap) ? -EIO : 0;
>
> case PTRACE_SETFPXREGS: /* Set the child extended FPU state. */
> return copy_regset_from_user(child, &user_x86_32_view,
> REGSET_XFP,
> 0, sizeof(struct user_fxsr_struct),
> - datap);
> + datap) ? -EIO : 0;
> #endif
>
> #if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
2008-07-01 9:02 ` Ingo Molnar
2008-07-01 9:32 ` Roland McGrath
@ 2008-07-03 2:37 ` TAKADA Yoshihito
1 sibling, 0 replies; 13+ messages in thread
From: TAKADA Yoshihito @ 2008-07-03 2:37 UTC (permalink / raw)
To: mingo; +Cc: roland, tglx, linux-kernel, stable
Hi.
The Roland's patch is bettor than mine.
Please apply Roland's only. Don't apply both. Either is enough.
From: Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
Date: Tue, 1 Jul 2008 11:02:04 +0200
>
> * Roland McGrath <roland@redhat.com> wrote:
>
> > ptrace has always returned only -EIO for all failures to access
> > registers. The user_regset calls are allowed to return a more
> > meaningful variety of errors. The REGSET_XFP calls use -ENODEV for
> > !cpu_has_fxsr hardware. Make ptrace return the traditional -EIO
> > instead of the error code from the user_regset call.
>
> since the original fix is already upstream, i've applied the delta patch
> below. Should we still do this for v2.6.26 or can we defer it to
> v2.6.27? As ptrace is the only user of this facility for now this would
> be an identity transformation AFAICS and the v2.6.26 release is very
> close.
>
> Ingo
>
> ---------------->
> Subject: x86 ptrace: fix PTRACE_GETFPXREGS error
> From: Roland McGrath <roland@redhat.com>
> Date: Mon, 30 Jun 2008 14:02:41 -0700 (PDT)
>
> ptrace has always returned only -EIO for all failures to access
> registers. The user_regset calls are allowed to return a more
> meaningful variety of errors. The REGSET_XFP calls use -ENODEV
> for !cpu_has_fxsr hardware. Make ptrace return the traditional
> -EIO instead of the error code from the user_regset call.
>
> Signed-off-by: Roland McGrath <roland@redhat.com>
> Cc: stable@kernel.org
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
> arch/x86/kernel/i387.c | 4 ++--
> arch/x86/kernel/ptrace.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> Index: tip/arch/x86/kernel/i387.c
> ===================================================================
> --- tip.orig/arch/x86/kernel/i387.c
> +++ tip/arch/x86/kernel/i387.c
> @@ -162,7 +162,7 @@ int xfpregs_get(struct task_struct *targ
> int ret;
>
> if (!cpu_has_fxsr)
> - return -EIO;
> + return -ENODEV;
>
> ret = init_fpu(target);
> if (ret)
> @@ -179,7 +179,7 @@ int xfpregs_set(struct task_struct *targ
> int ret;
>
> if (!cpu_has_fxsr)
> - return -EIO;
> + return -ENODEV;
>
> ret = init_fpu(target);
> if (ret)
> Index: tip/arch/x86/kernel/ptrace.c
> ===================================================================
> --- tip.orig/arch/x86/kernel/ptrace.c
> +++ tip/arch/x86/kernel/ptrace.c
> @@ -943,13 +943,13 @@ long arch_ptrace(struct task_struct *chi
> return copy_regset_to_user(child, &user_x86_32_view,
> REGSET_XFP,
> 0, sizeof(struct user_fxsr_struct),
> - datap);
> + datap) ? -EIO : 0;
>
> case PTRACE_SETFPXREGS: /* Set the child extended FPU state. */
> return copy_regset_from_user(child, &user_x86_32_view,
> REGSET_XFP,
> 0, sizeof(struct user_fxsr_struct),
> - datap);
> + datap) ? -EIO : 0;
> #endif
>
> #if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error
2008-07-03 1:58 ` TAKADA Yoshihito
@ 2008-07-03 3:00 ` Roland McGrath
0 siblings, 0 replies; 13+ messages in thread
From: Roland McGrath @ 2008-07-03 3:00 UTC (permalink / raw)
To: TAKADA Yoshihito; +Cc: mingo, tglx, linux-kernel, stable
> Hi. Thanks for your right patch.
> BTW, are FXSAVE and FXRSTOR instructions device?
> Is it right to return ENODEV?
> I think I had bettor return EXIO or ENOTSUP.
> If it discussed, tell me URL of tree at lkml.org.
I don't think it was discussed. It's clearly documented in the
linux/regset.h kerneldoc comments that define the interface.
I don't recall any feedback about error codes when that went in.
Device, eh, whatever. We're not going to add a new code that means
precisely "this user_regset refers to hardware not present", that would be
silly. It's going to be chosen by some analogy with the original use of
some existing code. ENXIO is fine too. I probably chose ENODEV over ENXIO
just because it's easier to read the name and remember what it might mean
(and it's easier to type).
All that really matters is that for the particular case of user_regset
functions, there be clearly specified one errno code that means "all's well,
but this supported hardware is not here". Then callers know to check for
that, and any other error code value is potentially an unexpected sort of
error. I see nothing wrong with ENODEV. The only thing wrong with ENXIO is
trying to type it correctly twice in a row. ENOTSUP does not seem like a
good fit as an analogy to its other specified uses.
Thanks,
Roland
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2008-07-03 7:07 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-30 4:44 [PATCH] ptrace GET/SET FPXREGS broken TAKADA Yoshihito
2008-06-30 12:18 ` Ingo Molnar
2008-06-30 21:02 ` [PATCH] x86 ptrace: fix PTRACE_GETFPXREGS error Roland McGrath
2008-07-01 9:02 ` Ingo Molnar
2008-07-01 9:32 ` Roland McGrath
2008-07-01 10:11 ` Ingo Molnar
2008-07-01 14:34 ` [stable] " Greg KH
2008-07-01 14:46 ` Ingo Molnar
2008-07-01 15:02 ` Greg KH
2008-07-03 2:37 ` TAKADA Yoshihito
2008-07-03 1:58 ` TAKADA Yoshihito
2008-07-03 3:00 ` Roland McGrath
2008-06-30 21:02 ` [PATCH] ptrace GET/SET FPXREGS broken Roland McGrath
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox