| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20061222060618.GD18827@slug>
Date: Fri, 22 Dec 2006 06:06:18 +0000
From: Frederik Deweerdt <deweerdt@...e.fr>
To: Andrew Morton <akpm@...l.org>
Cc: Jeremy Fitzhardinge <jeremy@...p.org>, linux-kernel@...r.kernel.org
Subject: Re: [-mm patch] ptrace: make {put,get}reg work again for gs and fs
On Thu, Dec 21, 2006 at 06:11:08PM -0800, Andrew Morton wrote:
> On Thu, 21 Dec 2006 18:00:49 -0800
> Jeremy Fitzhardinge <jeremy@...p.org> 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 <jeremy@...p.org>
>
> 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 <jeremy@...source.com>
> Cc: Frederik Deweerdt <deweerdt@...e.fr>
> Cc: Andi Kleen <ak@...e.de>
> Signed-off-by: Andrew Morton <akpm@...l.org>
> ---
>
> 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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists