Skip to contentSkip to conversation

x86-64 singlestep through sigreturn system call

PATCH Andi Kleen 3 participants 4 messages Jul 17, 2004 – Jul 22, 2004 2 messages not in the archive ptrace support

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

Andi Kleen

author
#2 lore raw
Quoted 10 lines · Roland McGrath
Quoted 3 lines

Anyways, even if I applied your patch there would be still inconsistency because there are several other system calls that use IRET. So I don't see much advantage in adding a special case just for sigreturn.

Now that I see that the difference is due to iret being used, I have a different solution that handles all cases. The following patch replaces both my previous patch for x86-64 native behavior and my patch for x86-64's ia32 support. This patch just directly clones Davide Libenzi's i386 code for x86-64 in both 64-bit and 32-bit cases. With this, the behavior of single-stepping all system calls is consistent.

Hmm, but now the behaviour for 32bit processes is different from the native 32bit kernel, since 32bit didn't apply any patch so far AFAIK.

If you send me a patch that just changes the behaviour of 64bit IRET I would apply it.

-Andi

#3 lore raw
Quoted 2 lines

Hmm, but now the behaviour for 32bit processes is different from the native 32bit kernel, since 32bit didn't apply any patch so far AFAIK.

Well, I was (and am) advocating that Davide's patch for native 32-bit be applied as well. If it is, then my last x86-64 patch covers both 32-bit and 64-bit cases with minimal cruft. In particular, 2.6.8-rc1-mm1 contains Davide's patch and both of my earlier x86-64 patches. I hope Andrew will replace those two patches with the later cleaner single x86-64 patch. If Linus merges that patch of mine and Davide's i386 patch, I will be a happy camper.

Quoted 2 lines

If you send me a patch that just changes the behaviour of 64bit IRET I would apply it.

Ok. This is just one line different from the patch I just posted. In 32-bit processes, some ptrace operations will superfluously clear a flag bit that can never be set, rather than test another flag bit to branch around doing it. If/when the native 32-bit behavior fix gets merged in and you want to match it, just remove `if (!test_tsk_thread_flag(child, TIF_IA32))' (which is the same as reverting this patch and applying the earlier one).

Thanks,
Roland

Index: linux-2.6/arch/x86_64/kernel/entry.S
===================================================================
RCS file: /home/roland/redhat/bkcvs/linux-2.5/arch/x86_64/kernel/entry.S,v retrieving revision 1.22

arch/x86_64/kernel/entry.S 15 Jul 2004 23:45:59 -0000 renamed from arch/x86_64/kernel/entry.S 12 Apr 2004 20:29:12 -0000 1.22 +1 -1
diff -b -p -u -r1.22 entry.S
@@ -297,7 +297,7 @@ int_very_careful:
 	sti
 	SAVE_REST
 	/* Check for syscall exit trace */	
-	testl $(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT),%edx
+	testl $(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SINGLESTEP),%edx
 	jz int_signal
 	pushq %rdi
 	leaq 8(%rsp),%rdi	# &ptregs -> arg1

Index: linux-2.6/ptrace.c
===================================================================
RCS file: /home/roland/redhat/bkcvs/linux-2.5/arch/x86_64/kernel/ptrace.c,v retrieving revision 1.16

ptrace.c 22 Jul 2004 00:42:06 -0000 renamed from ptrace.c 31 May 2004 03:07:42 -0000 1.16 +7 -1
diff -u -b -p -r1.16 ptrace.c
@@ -88,6 +88,7 @@ void ptrace_disable(struct task_struct *
 { 
 	long tmp;
 
+	clear_tsk_thread_flag(child, TIF_SINGLESTEP);
 	tmp = get_stack_long(child, EFL_OFFSET) & ~TRAP_FLAG;
 	put_stack_long(child, EFL_OFFSET, tmp);
 }

@@ -344,6 +345,7 @@ asmlinkage long sys_ptrace(long request,
 			set_tsk_thread_flag(child,TIF_SYSCALL_TRACE);
 		else
 			clear_tsk_thread_flag(child,TIF_SYSCALL_TRACE);
+		clear_tsk_thread_flag(child, TIF_SINGLESTEP);
 		child->exit_code = data;
 	/* make sure the single step bit is not set. */
 		tmp = get_stack_long(child, EFL_OFFSET);

@@ -395,6 +397,7 @@ asmlinkage long sys_ptrace(long request,
 		ret = 0;
 		if (child->state == TASK_ZOMBIE)	/* already dead */
 			break;
+		clear_tsk_thread_flag(child, TIF_SINGLESTEP);
 		child->exit_code = SIGKILL;
 		/* make sure the single step bit is not set. */
 		tmp = get_stack_long(child, EFL_OFFSET) & ~TRAP_FLAG;

@@ -416,6 +419,8 @@ asmlinkage long sys_ptrace(long request,
 		}
 		tmp = get_stack_long(child, EFL_OFFSET) | TRAP_FLAG;
 		put_stack_long(child, EFL_OFFSET, tmp);
+		if (!test_tsk_thread_flag(child, TIF_IA32))
+			set_tsk_thread_flag(child, TIF_SINGLESTEP);
 		child->exit_code = data;
 		/* give it a chance to run. */
 		wake_up_process(child);

@@ -528,7 +533,8 @@ asmlinkage void syscall_trace_leave(stru
 	if (unlikely(current->audit_context))
 		audit_syscall_exit(current, regs->rax);
 
-	if (test_thread_flag(TIF_SYSCALL_TRACE)
+	if ((test_thread_flag(TIF_SYSCALL_TRACE)
+	     || test_thread_flag(TIF_SINGLESTEP))
 	    && (current->ptrace & PT_PTRACED))
 		syscall_trace(regs);
 }
#4 lore raw
Quoted 11 lines · Roland McGrath
Quoted 3 lines

Hmm, but now the behaviour for 32bit processes is different from the native 32bit kernel, since 32bit didn't apply any patch so far AFAIK.

Well, I was (and am) advocating that Davide's patch for native 32-bit be applied as well. If it is, then my last x86-64 patch covers both 32-bit and 64-bit cases with minimal cruft. In particular, 2.6.8-rc1-mm1 contains Davide's patch and both of my earlier x86-64 patches. I hope Andrew will replace those two patches with the later cleaner single x86-64 patch. If Linus merges that patch of mine and Davide's i386 patch, I will be a happy camper.

Am now all confused. Please tell me which patches to drop, and ensure that I have the replacement patches, if any. Thanks.

#5 lore raw
Quoted 2 lines

Am now all confused. Please tell me which patches to drop, and ensure that I have the replacement patches, if any. Thanks.

Please drop these two:

	x86-64-singlestep-through-sigreturn-system-call.patch
	x86-64-support-for-singlestep-into-32-bit-system-calls.patch

and replace them with the following patch, which I first posted last week. With this, x86-64's behavior for ia32 matches the native i386 behavior given really-ptrace-single-step-2.patch, and x86-64's native behavior is similarly improved. The two patches above together do the same as this one alone, but this one is cleaner.

Thanks,
Roland

Index: linux-2.6/arch/x86_64/kernel/entry.S
===================================================================
RCS file: /home/roland/redhat/bkcvs/linux-2.5/arch/x86_64/kernel/entry.S,v retrieving revision 1.22

arch/x86_64/kernel/entry.S 15 Jul 2004 23:45:59 -0000 renamed from arch/x86_64/kernel/entry.S 12 Apr 2004 20:29:12 -0000 1.22 +1 -1
diff -b -p -u -r1.22 entry.S
@@ -297,7 +297,7 @@ int_very_careful:
 	sti
 	SAVE_REST
 	/* Check for syscall exit trace */	
-	testl $(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT),%edx
+	testl $(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SINGLESTEP),%edx
 	jz int_signal
 	pushq %rdi
 	leaq 8(%rsp),%rdi	# &ptregs -> arg1

Index: linux-2.6/arch/x86_64/kernel/ptrace.c
===================================================================
RCS file: /home/roland/redhat/bkcvs/linux-2.5/arch/x86_64/kernel/ptrace.c,v retrieving revision 1.16

arch/x86_64/kernel/ptrace.c 15 Jul 2004 23:56:44 -0000 renamed from arch/x86_64/kernel/ptrace.c 31 May 2004 03:07:42 -0000 1.16 +6 -1
diff -b -p -u -r1.16 ptrace.c
@@ -88,6 +88,7 @@ void ptrace_disable(struct task_struct *
 { 
 	long tmp;
 
+	clear_tsk_thread_flag(child, TIF_SINGLESTEP);
 	tmp = get_stack_long(child, EFL_OFFSET) & ~TRAP_FLAG;
 	put_stack_long(child, EFL_OFFSET, tmp);
 }

@@ -344,6 +345,7 @@ asmlinkage long sys_ptrace(long request,
 			set_tsk_thread_flag(child,TIF_SYSCALL_TRACE);
 		else
 			clear_tsk_thread_flag(child,TIF_SYSCALL_TRACE);
+		clear_tsk_thread_flag(child, TIF_SINGLESTEP);
 		child->exit_code = data;
 	/* make sure the single step bit is not set. */
 		tmp = get_stack_long(child, EFL_OFFSET);

@@ -395,6 +397,7 @@ asmlinkage long sys_ptrace(long request,
 		ret = 0;
 		if (child->state == TASK_ZOMBIE)	/* already dead */
 			break;
+		clear_tsk_thread_flag(child, TIF_SINGLESTEP);
 		child->exit_code = SIGKILL;
 		/* make sure the single step bit is not set. */
 		tmp = get_stack_long(child, EFL_OFFSET) & ~TRAP_FLAG;

@@ -416,6 +419,7 @@ asmlinkage long sys_ptrace(long request,
 		}
 		tmp = get_stack_long(child, EFL_OFFSET) | TRAP_FLAG;
 		put_stack_long(child, EFL_OFFSET, tmp);
+		set_tsk_thread_flag(child, TIF_SINGLESTEP);
 		child->exit_code = data;
 		/* give it a chance to run. */
 		wake_up_process(child);

@@ -528,7 +532,8 @@ asmlinkage void syscall_trace_leave(stru
 	if (unlikely(current->audit_context))
 		audit_syscall_exit(current, regs->rax);
 
-	if (test_thread_flag(TIF_SYSCALL_TRACE)
+	if ((test_thread_flag(TIF_SYSCALL_TRACE)
+	     || test_thread_flag(TIF_SINGLESTEP))
 	    && (current->ptrace & PT_PTRACED))
 		syscall_trace(regs);
 }