2006-02-17 00:14:35

by Chuck Ebbert

[permalink] [raw]
Subject: [patch] i386: fix singlestepping though a syscall

Singlestep through a syscall using vsyscall-sysenter had two bugs:

1. Setting TIF_SINGLESTEP is not enough to force
do_notify_resume() to be run on return to user;
TIF_IRET must also be set.

2. do_notify_resume() was being passed masked copies
of task_thread_flags, so TIF_SINGLESTEP was never
seen as set when it was called. This was changed
to use the 'test' instruction instead of 'and';
a duplicated piece of code was removed instead
of fixing that part too.

Also changed some misleading 'jne' to 'jnz' to make it
clearer what is happening.

Signed-off-by: Chuck Ebbert <[email protected]>

---
arch/i386/kernel/entry.S | 17 +++++------------
arch/i386/kernel/traps.c | 1 +
2 files changed, 6 insertions(+), 12 deletions(-)

--- 2.6.16-rc3-nb.orig/arch/i386/kernel/entry.S
+++ 2.6.16-rc3-nb/arch/i386/kernel/entry.S
@@ -152,9 +152,9 @@ ENTRY(resume_userspace)
# setting need_resched or sigpending
# between sampling and the iret
movl TI_flags(%ebp), %ecx
- andl $_TIF_WORK_MASK, %ecx # is there any work to be done on
+ testl $_TIF_WORK_MASK, %ecx # is there any work to be done on
# int/exception return?
- jne work_pending
+ jnz work_pending
jmp restore_all

#ifdef CONFIG_PREEMPT
@@ -301,21 +301,14 @@ work_pending:
jz work_notifysig
work_resched:
call schedule
- cli # make sure we don't miss an interrupt
- # setting need_resched or sigpending
- # between sampling and the iret
- movl TI_flags(%ebp), %ecx
- andl $_TIF_WORK_MASK, %ecx # is there any work to be done other
- # than syscall tracing?
- jz restore_all
- testb $_TIF_NEED_RESCHED, %cl
- jnz work_resched
+ jmp resume_userspace

+ ALIGN
work_notifysig: # deal with pending signals and
# notify-resume requests
testl $VM_MASK, EFLAGS(%esp)
movl %esp, %eax
- jne work_notifysig_v86 # returning to kernel-space or
+ jnz work_notifysig_v86 # returning to kernel-space or
# vm86-space
xorl %edx, %edx
call do_notify_resume
--- 2.6.16-rc3-nb.orig/arch/i386/kernel/traps.c
+++ 2.6.16-rc3-nb/arch/i386/kernel/traps.c
@@ -795,6 +795,7 @@ debug_vm86:

clear_TF_reenable:
set_tsk_thread_flag(tsk, TIF_SINGLESTEP);
+ set_tsk_thread_flag(tsk, TIF_IRET);
regs->eflags &= ~TF_MASK;
return;
}
--
Chuck
"Equations are the Devil's sentences." --Stephen Colbert


2006-02-17 01:06:42

by Linus Torvalds

[permalink] [raw]
Subject: Re: [patch] i386: fix singlestepping though a syscall



On Thu, 16 Feb 2006, Chuck Ebbert wrote:
>
> Singlestep through a syscall using vsyscall-sysenter had two bugs:
>
> 1. Setting TIF_SINGLESTEP is not enough to force
> do_notify_resume() to be run on return to user;
> TIF_IRET must also be set.

Interesting, but I think you pinpointed the _real_ bug.

TIF_SINGLESTEP very much should be enough to force do_notify_resume() to
be run on return to user space.

Sounds like somebody is testing _TIF_WORK_MASK rather than
_TIF_ALLWORK_MASK.

I'd suspect the "work_pending" case. Looks like we miss testing the TIF
flags there.

Oh, actually, I think you should just remove the clearing of
_TIF_SINGLESTEP from _TIF_WORK_MASK. Does that fix the bug.

Linus

2006-02-17 08:20:52

by Chuck Ebbert

[permalink] [raw]
Subject: Re: [patch] i386: fix singlestepping though a syscall

On Thu, 16 Feb 2006 at 17:06:22 -0800, Linus Torvalds wrote:

> On Thu, 16 Feb 2006, Chuck Ebbert wrote:
> >
> > Singlestep through a syscall using vsyscall-sysenter had two bugs:
> >
> > 1. Setting TIF_SINGLESTEP is not enough to force
> > do_notify_resume() to be run on return to user;
> > TIF_IRET must also be set.
>
> Interesting, but I think you pinpointed the _real_ bug.
> ...
> Oh, actually, I think you should just remove the clearing of
> _TIF_SINGLESTEP from _TIF_WORK_MASK. Does that fix the bug.

Yes, that works. I was afraid to try it because of unknown side-effects
but one of those may be that it fixes Paolo's debugger problem. The C
program I was testing with is enclosed and I can even (with care) debug
it with gdb passing through SIGTRAP and step through the signal handler.

Paolo, can you try this patch for your debugger problem?


Do not mask TIF_SINGLESTEP bit in _TIF_WORK_MASK. Masking this stopped
do_notify_resume() from being called when it should have been.

Signed-off-by: Chuck Ebbert <[email protected]>

--- 2.6.16-rc3-nb.orig/include/asm-i386/thread_info.h
+++ 2.6.16-rc3-nb/include/asm-i386/thread_info.h
@@ -158,8 +158,8 @@ register unsigned long current_stack_poi

/* work to do on interrupt/exception return */
#define _TIF_WORK_MASK \
- (0x0000FFFF & ~(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SINGLESTEP|\
- _TIF_SECCOMP|_TIF_SYSCALL_EMU))
+ (0x0000FFFF & ~(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
+ _TIF_SECCOMP | _TIF_SYSCALL_EMU))
/* work to do on any return to u-space */
#define _TIF_ALLWORK_MASK (0x0000FFFF & ~_TIF_SECCOMP)
_

#define _GNU_SOURCE
#include <stdlib.h>
#include <unistd.h>
#include <stdio.h>
#include <signal.h>
#include <errno.h>

#define TRAP_FLAG 0x100

#ifdef INT80
# define ENTER_KERNEL "int $0x80\n\t"
#else
# define ENTER_KERNEL "call *vsyscall\n\t"
#endif

static struct sigaction sa;
static void * const vsyscall = (void *)0xffffe400;

static void handler(int nr, siginfo_t *si, void *vuc)
{
struct ucontext *uc = (struct ucontext *)vuc;
struct sigcontext *sc = (struct sigcontext *)&uc->uc_mcontext;

printf("handler: signo = %d, errno = %d, code = %d\n",
si->si_signo, si->si_errno, si->si_code);
printf("handler: addr = 0x%08x, inst = 0x%02x, flags=0x%08x\n\n",
si->si_addr, *(unsigned char *)si->si_addr, sc->eflags);
}

int main(int argc, char * const argv[])
{
sa.sa_sigaction = handler;
sa.sa_flags = SA_SIGINFO;
sigaction(SIGTRAP, &sa, NULL);

asm volatile ("pushf ; orl %0,(%%esp) ; popf" : : "i" (TRAP_FLAG));
asm volatile (ENTER_KERNEL : : "a" (20) /* getpid() */);
asm volatile ("pushf ; andl %0,(%%esp) ; popf" : : "i" (~TRAP_FLAG));
asm volatile (ENTER_KERNEL : : "a" (20) /* getpid() */);

return 0;
}
--
Chuck
"Equations are the Devil's sentences." --Stephen Colbert

2006-02-17 15:19:40

by Paulo Marques

[permalink] [raw]
Subject: Re: [patch] i386: fix singlestepping though a syscall

Chuck Ebbert wrote:
>[...]
> Yes, that works. I was afraid to try it because of unknown side-effects
> but one of those may be that it fixes Paolo's debugger problem. The C
> program I was testing with is enclosed and I can even (with care) debug
> it with gdb passing through SIGTRAP and step through the signal handler.
>
> Paolo, can you try this patch for your debugger problem?
^
Paulo, please ;)

Humm.... I've already found a way to make the debugger work (see th
other thread) and at a first glance it seems unrelated to this. So I
suspect I wouldn't gain much by testing it.

If you think it is worth doing it anyway, it won't be that much trouble
to test yet another kernel, so I can certainly do it tonight...

--
Paulo Marques - http://www.grupopie.com

Pointy-Haired Boss: I don't see anything that could stand in our way.
Dilbert: Sanity? Reality? The laws of physics?