Re: Crash when attaching uretprobes to processes running in Docker
From: Jiri Olsa <hidden>
Date: 2025-01-14 09:22:25
Also in:
bpf, linux-api, lkml
Subsystem:
performance events subsystem, the rest, uprobes, x86 architecture (32-bit and 64-bit) · Maintainers:
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Linus Torvalds, Masami Hiramatsu, Oleg Nesterov, Thomas Gleixner, Borislav Petkov, Dave Hansen
On Sat, Jan 11, 2025 at 07:40:15PM +0100, Jiri Olsa wrote:
On Sat, Jan 11, 2025 at 02:25:37AM +1100, Aleksa Sarai wrote:quoted
On 2025-01-10, Eyal Birger [off-list ref] wrote:quoted
Hi, When attaching uretprobes to processes running inside docker, the attached process is segfaulted when encountering the retprobe. The offending commit is: ff474a78cef5 ("uprobe: Add uretprobe syscall to speed up return probe") To my understanding, the reason is that now that uretprobe is a system call, the default seccomp filters in docker block it as they only allow a specific set of known syscalls.FWIW, the default seccomp profile of Docker _should_ return -ENOSYS for uretprobe (runc has a bunch of ugly logic to try to guarantee this if Docker hasn't updated their profile to include it). Though I guess that isn't sufficient for the magic that uretprobe(2) does...quoted
This behavior can be reproduced by the below bash script, which works before this commit. Reported-by: Rafael Buchbinder <redacted>hi, nice ;-) thanks for the report, the problem seems to be that uretprobe syscall is blocked and uretprobe trampoline does not expect that I think we could add code to the uretprobe trampoline to detect this and execute standard int3 as fallback to process uretprobe, I'm checking on that
hack below seems to fix the issue, it's using rbx to signal that uretprobe syscall got executed, if not, trampoline does int3 and executes uretprobe handler in the old way unfortunately now the uretprobe trampoline size crosses the xol slot limit so will need to come up with some generic/arch code solution for that, code below is neglecting that for now jirka --- arch/x86/kernel/uprobes.c | 24 ++++++++++++++++++++++++ include/linux/uprobes.h | 1 + kernel/events/uprobes.c | 10 ++++++++-- 3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 5a952c5ea66b..b54863f6fa25 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c@@ -315,14 +315,25 @@ asm ( ".global uretprobe_trampoline_entry\n" "uretprobe_trampoline_entry:\n" "pushq %rax\n" + "pushq %rbx\n" "pushq %rcx\n" "pushq %r11\n" + "movq $1, %rbx\n" "movq $" __stringify(__NR_uretprobe) ", %rax\n" "syscall\n" ".global uretprobe_syscall_check\n" "uretprobe_syscall_check:\n" + "or %rbx,%rbx\n" + "jz uretprobe_syscall_return\n" "popq %r11\n" "popq %rcx\n" + "popq %rbx\n" + "popq %rax\n" + "int3\n" + "uretprobe_syscall_return:\n" + "popq %r11\n" + "popq %rcx\n" + "popq %rbx\n" /* The uretprobe syscall replaces stored %rax value with final * return address, so we don't restore %rax in here and just
@@ -338,6 +349,16 @@ extern u8 uretprobe_trampoline_entry[]; extern u8 uretprobe_trampoline_end[]; extern u8 uretprobe_syscall_check[]; +#define UINSNS_PER_PAGE (PAGE_SIZE/UPROBE_XOL_SLOT_BYTES) + +bool arch_is_uretprobe_trampoline(unsigned long vaddr) +{ + unsigned long start = uprobe_get_trampoline_vaddr(); + unsigned long end = start + 2*UINSNS_PER_PAGE; + + return vaddr >= start && vaddr < end; +} + void *arch_uprobe_trampoline(unsigned long *psize) { static uprobe_opcode_t insn = UPROBE_SWBP_INSN;
@@ -418,6 +439,9 @@ SYSCALL_DEFINE0(uretprobe) regs->r11 = regs->flags; regs->cx = regs->ip; + /* zero rbx to signal trampoline that uretprobe syscall was executed */ + regs->bx = 0; + return regs->ax; sigill:
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index e0a4c2082245..dbde57a68a1b 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h@@ -213,6 +213,7 @@ extern void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr, extern void uprobe_handle_trampoline(struct pt_regs *regs); extern void *arch_uprobe_trampoline(unsigned long *psize); extern unsigned long uprobe_get_trampoline_vaddr(void); +bool arch_is_uretprobe_trampoline(unsigned long vaddr); #else /* !CONFIG_UPROBES */ struct uprobes_state { };
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index fa04b14a7d72..73df64109f38 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c@@ -1703,6 +1703,11 @@ void * __weak arch_uprobe_trampoline(unsigned long *psize) return &insn; } +bool __weak arch_is_uretprobe_trampoline(unsigned long vaddr) +{ + return vaddr == uprobe_get_trampoline_vaddr(); +} + static struct xol_area *__create_xol_area(unsigned long vaddr) { struct mm_struct *mm = current->mm;
@@ -1725,8 +1730,9 @@ static struct xol_area *__create_xol_area(unsigned long vaddr) area->vaddr = vaddr; init_waitqueue_head(&area->wq); - /* Reserve the 1st slot for get_trampoline_vaddr() */ + /* Reserve the first two slots for get_trampoline_vaddr() */ set_bit(0, area->bitmap); + set_bit(1, area->bitmap); insns = arch_uprobe_trampoline(&insns_size); arch_uprobe_copy_ixol(area->page, 0, insns, insns_size);
@@ -2536,7 +2542,7 @@ static void handle_swbp(struct pt_regs *regs) int is_swbp; bp_vaddr = uprobe_get_swbp_addr(regs); - if (bp_vaddr == uprobe_get_trampoline_vaddr()) + if (arch_is_uretprobe_trampoline(bp_vaddr)) return uprobe_handle_trampoline(regs); rcu_read_lock_trace();
--
2.47.1