On Thu, Dec 21, 2006 at 06:11:08PM -0800, Andrew Morton wrote: > On Thu, 21 Dec 2006 18:00:49 -0800 > Jeremy Fitzhardinge <[EMAIL PROTECTED]> wrote: > > > Frederik Deweerdt wrote: > > > This is a -mm1 kernel + your efl_offset fix + the attached patch. > > > So the problem came from putreg still saving %gs to the stack where > > > there's no slot for it, whereas getreg got things right. > > > > > > > That patch looks good, but I think it is already effectively in Andrew's > > queue, because I noticed some problems in there when I reviewed the > > convert-to-%fs patch. > > > > The below is what I have queued for urgent mainlining to address these > problems. > > Is it sufficient? > No, it's not. The patch below fixes the place where we get eflags, this triggered the "BUG while gdb'ing" reports. The one I sent was to fix a problem that only I reported, AFAIK: when you use gdb/ptrace to modify %fs, the value gets written in the wrong place (see gdb sessions). So, unless you have another patch fixing the way putreg() writes %fs, the patch[1] I sent should also be queued for mainline.
Regards, Frederik [1] http://lkml.org/lkml/2006/12/21/267 > > > > From: Jeremy Fitzhardinge <[EMAIL PROTECTED]> > > The PDA patches introduced a bug in ptrace: it reads eflags from the wrong > place on the target's stack, but writes it back to the correct place. The > result is a corrupted eflags, which is most visible when it turns interrupts > off unexpectedly. > > This patch fixes this by making the ptrace code a little less fragile. It > changes [gs]et_stack_long to take a straightforward byte offset into struct > pt_regs, rather than requiring all callers to do a sizeof(struct pt_regs) > offset adjustment. This means that the eflag's offset (EFL_OFFSET) on the > target stack can be simply computed with offsetof(). > > Signed-off-by: Jeremy Fitzhardinge <[EMAIL PROTECTED]> > Cc: Frederik Deweerdt <[EMAIL PROTECTED]> > Cc: Andi Kleen <[EMAIL PROTECTED]> > Signed-off-by: Andrew Morton <[EMAIL PROTECTED]> > --- > > arch/i386/kernel/ptrace.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff -puN > arch/i386/kernel/ptrace.c~ptrace-fix-efl_offset-value-according-to-i386-pda-changes > arch/i386/kernel/ptrace.c > --- > a/arch/i386/kernel/ptrace.c~ptrace-fix-efl_offset-value-according-to-i386-pda-changes > +++ a/arch/i386/kernel/ptrace.c > @@ -45,7 +45,7 @@ > /* > * Offset of eflags on child stack.. > */ > -#define EFL_OFFSET ((EFL-2)*4-sizeof(struct pt_regs)) > +#define EFL_OFFSET offsetof(struct pt_regs, eflags) > > static inline struct pt_regs *get_child_regs(struct task_struct *task) > { > @@ -54,24 +54,24 @@ static inline struct pt_regs *get_child_ > } > > /* > - * this routine will get a word off of the processes privileged stack. > - * the offset is how far from the base addr as stored in the TSS. > - * this routine assumes that all the privileged stacks are in our > + * This routine will get a word off of the processes privileged stack. > + * the offset is bytes into the pt_regs structure on the stack. > + * This routine assumes that all the privileged stacks are in our > * data space. > */ > static inline int get_stack_long(struct task_struct *task, int offset) > { > unsigned char *stack; > > - stack = (unsigned char *)task->thread.esp0; > + stack = (unsigned char *)task->thread.esp0 - sizeof(struct pt_regs); > stack += offset; > return (*((int *)stack)); > } > > /* > - * this routine will put a word on the processes privileged stack. > - * the offset is how far from the base addr as stored in the TSS. > - * this routine assumes that all the privileged stacks are in our > + * This routine will put a word on the processes privileged stack. > + * the offset is bytes into the pt_regs structure on the stack. > + * This routine assumes that all the privileged stacks are in our > * data space. > */ > static inline int put_stack_long(struct task_struct *task, int offset, > @@ -79,7 +79,7 @@ static inline int put_stack_long(struct > { > unsigned char * stack; > > - stack = (unsigned char *) task->thread.esp0; > + stack = (unsigned char *)task->thread.esp0 - sizeof(struct pt_regs); > stack += offset; > *(unsigned long *) stack = data; > return 0; > @@ -114,7 +114,7 @@ static int putreg(struct task_struct *ch > } > if (regno > ES*4) > regno -= 1*4; > - put_stack_long(child, regno - sizeof(struct pt_regs), value); > + put_stack_long(child, regno, value); > return 0; > } > > @@ -137,7 +137,6 @@ static unsigned long getreg(struct task_ > default: > if (regno > ES*4) > regno -= 1*4; > - regno = regno - sizeof(struct pt_regs); > retval &= get_stack_long(child, regno); > } > return retval; > _ > > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/