Re: [patch] i386: fix singlestepping though a syscall

Re: [patch] i386: fix singlestepping though a syscall

[Chuck Ebbert]

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 <76306.1226@compuserve.com>

--- 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

Re: [patch] i386: fix singlestepping though a syscall

[Paulo Marques]

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 - www.grupopie.com

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

Re: [patch] i386: fix singlestepping though a syscall

[Linus Torvalds]

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

[patch] i386: fix singlestepping though a syscall

[Chuck Ebbert]

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 <76306.1226@compuserve.com>

---
 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