On Tue, Feb 11, 2025 at 8:48 AM Andrii Nakryiko andrii.nakryiko@gmail.com wrote:
On Tue, Feb 11, 2025 at 3:16 AM Jiri Olsa jolsa@kernel.org wrote:
Jann reported [1] possible issue when trampoline_check_ip returns address near the bottom of the address space that is allowed to call into the syscall if uretprobes are not set up.
Though the mmap minimum address restrictions will typically prevent creating mappings there, let's make sure uretprobe syscall checks for that.
[1] https://lore.kernel.org/bpf/202502081235.5A6F352985@keescook/T/#m9d416df341b... Cc: Kees Cook kees@kernel.org Cc: Eyal Birger eyal.birger@gmail.com Cc: stable@vger.kernel.org Fixes: ff474a78cef5 ("uprobe: Add uretprobe syscall to speed up return probe") Reported-by: Jann Horn jannh@google.com Reviewed-by: Oleg Nesterov oleg@redhat.com Reviewed-by: Kees Cook kees@kernel.org Signed-off-by: Jiri Olsa jolsa@kernel.org
v2 changes:
- adding UPROBE_NO_TRAMPOLINE_VADDR macro (Andrii)
- rebased on top of perf/core
arch/x86/kernel/uprobes.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c index 5a952c5ea66b..e8d3c59aa9f7 100644 --- a/arch/x86/kernel/uprobes.c +++ b/arch/x86/kernel/uprobes.c @@ -357,19 +357,25 @@ void *arch_uprobe_trampoline(unsigned long *psize) return &insn; }
-static unsigned long trampoline_check_ip(void) +static unsigned long trampoline_check_ip(unsigned long tramp) {
unsigned long tramp = uprobe_get_trampoline_vaddr();return tramp + (uretprobe_syscall_check - uretprobe_trampoline_entry);}
+#define UPROBE_NO_TRAMPOLINE_VADDR ((unsigned long)-1)
If you respin anyway maybe use ~0UL instead? In the above and in uprobe_get_trampoline_vaddr(), since
unsigned long trampoline_vaddr = -1;
looks odd too.