uprobes: change handle_swbp() to send SIGTRAP with si_code=SI_KERNEL

PATCH Applied Oleg Nesterov 3 participants 3 messages Jul 23, 2020 – Jul 24, 2020

Subsystems (3)

Stable branch perf events uprobes

lore

Share & tools

lore

Oleg Nesterov

author maintainer

2020-07-23 15:44 UTC #0 lore raw

PATCH 1/1 uprobes: change handle_swbp() to send SIGTRAP with si_code=SI_KERNEL

If a tracee is uprobed and it hits int3 inserted by debugger, handle_swbp() does send_sig(SIGTRAP, current, 0) which means si_code == SI_USER. This used to work when this code was written, but then GDB started to validate si_code and now it simply can't use breakpoints if the tracee has an active uprobe:

	# cat test.c
	void unused_func(void)
	{
	}
	int main(void)
	{
		return 0;
	}

	# gcc -g test.c -o test
	# perf probe -x ./test -a unused_func
	# perf record -e probe_test:unused_func gdb ./test -ex run
	GNU gdb (GDB) 10.0.50.20200714-git
	...
	Program received signal SIGTRAP, Trace/breakpoint trap.
	0x00007ffff7ddf909 in dl_main () from /lib64/ld-linux-x86-64.so.2
	(gdb)

The tracee hits the internal breakpoint inserted by GDB to monitor shared library events but GDB misinterprets this SIGTRAP and reports a signal.

Change handle_swbp() to use force_sig(SIGTRAP), this matches do_int3_user() and fixes the problem.

This is the minimal fix for -stable, arch/x86/kernel/uprobes.c is equally wrong; it should use send_sigtrap(TRAP_TRACE) instead of send_sig(SIGTRAP), but this doesn't confuse GDB and needs another x86-specific patch.

Reported-by Aaron Merey <[email protected]>

Cc [email protected]

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/events/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/events/uprobes.c +1 -1

index bb0862873dba..5f8b0c52fd2e 100644
@@ -2199,7 +2199,7 @@ static void handle_swbp(struct pt_regs *regs)
 	if (!uprobe) {
 		if (is_swbp > 0) {
 			/* No matching uprobe; signal SIGTRAP. */
-			send_sig(SIGTRAP, current, 0);
+			force_sig(SIGTRAP);
 		} else {
 			/*
 			 * Either we raced with uprobe_unregister() or we can't

Signature · 1 line

2.25.1.362.g51ebf55

Srikar Dronamraju

Reviewed-by on v1 1/1

2020-07-24 07:41 UTC #1 lore raw

Quoted 12 lines · Oleg Nesterov

If a tracee is uprobed and it hits int3 inserted by debugger, handle_swbp() does send_sig(SIGTRAP, current, 0) which means si_code == SI_USER. This used to work when this code was written, but then GDB started to validate si_code and now it simply can't use breakpoints if the tracee has an active uprobe:
The tracee hits the internal breakpoint inserted by GDB to monitor shared library events but GDB misinterprets this SIGTRAP and reports a signal.
Change handle_swbp() to use force_sig(SIGTRAP), this matches do_int3_user() and fixes the problem.

Quoted 7 lines

This is the minimal fix for -stable, arch/x86/kernel/uprobes.c is equally wrong; it should use send_sigtrap(TRAP_TRACE) instead of send_sig(SIGTRAP), but this doesn't confuse GDB and needs another x86-specific patch.
Reported-by Aaron Merey <[email protected]>
Cc [email protected]
Signed-off-by Oleg Nesterov <[email protected]>

Looks good to me.

Reviewed-by Srikar Dronamraju <[email protected]>

Quoted 21 lines

---
 kernel/events/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/events/uprobes.c +1 -1

index bb0862873dba..5f8b0c52fd2e 100644
@@ -2199,7 +2199,7 @@ static void handle_swbp(struct pt_regs *regs)
 	if (!uprobe) {
 		if (is_swbp > 0) {
 			/* No matching uprobe; signal SIGTRAP. */
-			send_sig(SIGTRAP, current, 0);
+			force_sig(SIGTRAP);
 		} else {
 			/*
 			 * Either we raced with uprobe_unregister() or we can't

Signature · 1 line

2.25.1.362.g51ebf55

Signature · 2 lines

Thanks and Regards
Srikar Dronamraju

tip-bot2 for Oleg Nesterov

merge bot Applied

2020-07-24 14:07 UTC #2 lore raw

[tip: perf/urgent] uprobes: Change handle_swbp() to send SIGTRAP with si_code=SI_KERNEL, to fix GDB regression

In reply to Oleg Nesterov

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     fe5ed7ab99c656bd2f5b79b49df0e9ebf2cead8a
Gitweb:        https://git.kernel.org/tip/fe5ed7ab99c656bd2f5b79b49df0e9ebf2cead8a
Author:        Oleg Nesterov <[email protected]>
AuthorDate:    Thu, 23 Jul 2020 17:44:20 +02:00
Committer:     Ingo Molnar <[email protected]>
CommitterDate: Fri, 24 Jul 2020 15:38:37 +02:00

uprobes: Change handle_swbp() to send SIGTRAP with si_code=SI_KERNEL, to fix GDB regression

	# cat test.c
	void unused_func(void)
	{
	}
	int main(void)
	{
		return 0;
	}

	# gcc -g test.c -o test
	# perf probe -x ./test -a unused_func
	# perf record -e probe_test:unused_func gdb ./test -ex run
	GNU gdb (GDB) 10.0.50.20200714-git
	...
	Program received signal SIGTRAP, Trace/breakpoint trap.
	0x00007ffff7ddf909 in dl_main () from /lib64/ld-linux-x86-64.so.2
	(gdb)

The tracee hits the internal breakpoint inserted by GDB to monitor shared library events but GDB misinterprets this SIGTRAP and reports a signal.

Change handle_swbp() to use force_sig(SIGTRAP), this matches do_int3_user() and fixes the problem.

Reported-by Aaron Merey <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>

Reviewed-by Srikar Dronamraju <[email protected]>

Cc [email protected]

Link https://lore.kernel.org/r/[email protected] open here

---
 kernel/events/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/events/uprobes.c +1 -1

index bb08628..5f8b0c5 100644
@@ -2199,7 +2199,7 @@ static void handle_swbp(struct pt_regs *regs)
 	if (!uprobe) {
 		if (is_swbp > 0) {
 			/* No matching uprobe; signal SIGTRAP. */
-			send_sig(SIGTRAP, current, 0);
+			force_sig(SIGTRAP);
 		} else {
 			/*
 			 * Either we raced with uprobe_unregister() or we can't