| Message ID | 20250202162921.335813-1-eyal.birger@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | seccomp: pass uretprobe system call through seccomp | expand |
On Sun, 02 Feb 2025 08:29:19 -0800, Eyal Birger wrote: > uretprobe(2) is an performance enhancement system call added to improve > uretprobes on x86_64. > > Confinement environments such as Docker are not aware of this new system > call and kill confined processes when uretprobes are attached to them. > > Since uretprobe is a "kernel implementation detail" system call which is > not used by userspace application code directly, pass this system call > through seccomp without forcing existing userspace confinement environments > to be changed. > > [...] With the changes I mentioned in each patch, I've applied this to for-next/seccomp, with the intention of getting them into v6.14-rc2. Thanks! [1/2] seccomp: passthrough uretprobe systemcall without filtering https://git.kernel.org/kees/c/cf6cb56ef244 [2/2] selftests/seccomp: validate uretprobe syscall passes through seccomp https://git.kernel.org/kees/c/c2debdb8544f Take care,
On Thu, Feb 6, 2025 at 1:22 PM Kees Cook <kees@kernel.org> wrote: > > On Sun, 02 Feb 2025 08:29:19 -0800, Eyal Birger wrote: > > uretprobe(2) is an performance enhancement system call added to improve > > uretprobes on x86_64. > > > > Confinement environments such as Docker are not aware of this new system > > call and kill confined processes when uretprobes are attached to them. > > > > Since uretprobe is a "kernel implementation detail" system call which is > > not used by userspace application code directly, pass this system call > > through seccomp without forcing existing userspace confinement environments > > to be changed. > > > > [...] > > With the changes I mentioned in each patch, I've applied this to > for-next/seccomp, with the intention of getting them into v6.14-rc2. > > Thanks! Thank you very much for your help. Eyal.
On Thu, Feb 06, 2025 at 05:06:29PM -0800, Eyal Birger wrote: > On Thu, Feb 6, 2025 at 1:22 PM Kees Cook <kees@kernel.org> wrote: > > > > On Sun, 02 Feb 2025 08:29:19 -0800, Eyal Birger wrote: > > > uretprobe(2) is an performance enhancement system call added to improve > > > uretprobes on x86_64. > > > > > > Confinement environments such as Docker are not aware of this new system > > > call and kill confined processes when uretprobes are attached to them. > > > > > > Since uretprobe is a "kernel implementation detail" system call which is > > > not used by userspace application code directly, pass this system call > > > through seccomp without forcing existing userspace confinement environments > > > to be changed. > > > > > > [...] > > > > With the changes I mentioned in each patch, I've applied this to > > for-next/seccomp, with the intention of getting them into v6.14-rc2. > > > > Thanks! > > Thank you very much for your help. great! thanks, jirka
On Sun, Feb 2, 2025 at 5:29 PM Eyal Birger <eyal.birger@gmail.com> wrote: > uretprobe(2) is an performance enhancement system call added to improve > uretprobes on x86_64. > > Confinement environments such as Docker are not aware of this new system > call and kill confined processes when uretprobes are attached to them. FYI, you might have similar issues with Syscall User Dispatch (https://docs.kernel.org/admin-guide/syscall-user-dispatch.html) and potentially also with ptrace-based sandboxes, depending on what kinda processes you inject uprobes into. For Syscall User Dispatch, there is already precedent for a bypass based on instruction pointer (see syscall_user_dispatch()). > Since uretprobe is a "kernel implementation detail" system call which is > not used by userspace application code directly, pass this system call > through seccomp without forcing existing userspace confinement environments > to be changed. This makes me feel kinda uncomfortable. The purpose of seccomp() is that you can create a process that is as locked down as you want; you can use it for some light limits on what a process can do (like in Docker), or you can use it to make a process that has access to essentially nothing except read(), write() and exit_group(). Even stuff like restart_syscall() and rt_sigreturn() is not currently excepted from that. I guess your usecase is a little special in that you were already calling from userspace into the kernel with SWBP before, which is also not subject to seccomp; and the syscall is essentially an arch-specific hack to make the SWBP a little faster. If we do this, we should at least ensure that there is absolutely no way for anything to happen in sys_uretprobe when no uretprobes are configured for the process - the first check in the syscall implementation almost does that, but the implementation could be a bit stricter. It checks for "regs->ip != trampoline_check_ip()", but if no uprobe region exists for the process, trampoline_check_ip() returns `-1 + (uretprobe_syscall_check - uretprobe_trampoline_entry)`. So there is a userspace instruction pointer 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, and uprobe_handle_trampoline() will SIGILL us if we get that far without a valid uretprobe.
Hi, On Fri, Feb 7, 2025 at 7:27 AM Jann Horn <jannh@google.com> wrote: > > On Sun, Feb 2, 2025 at 5:29 PM Eyal Birger <eyal.birger@gmail.com> wrote: > > uretprobe(2) is an performance enhancement system call added to improve > > uretprobes on x86_64. > > > > Confinement environments such as Docker are not aware of this new system > > call and kill confined processes when uretprobes are attached to them. > > FYI, you might have similar issues with Syscall User Dispatch > (https://docs.kernel.org/admin-guide/syscall-user-dispatch.html) and > potentially also with ptrace-based sandboxes, depending on what kinda > processes you inject uprobes into. For Syscall User Dispatch, there is > already precedent for a bypass based on instruction pointer (see > syscall_user_dispatch()). Thanks. This is interesting. Do you know of confinement environments using this? > > > Since uretprobe is a "kernel implementation detail" system call which is > > not used by userspace application code directly, pass this system call > > through seccomp without forcing existing userspace confinement environments > > to be changed. > > This makes me feel kinda uncomfortable. The purpose of seccomp() is > that you can create a process that is as locked down as you want; you > can use it for some light limits on what a process can do (like in > Docker), or you can use it to make a process that has access to > essentially nothing except read(), write() and exit_group(). Even > stuff like restart_syscall() and rt_sigreturn() is not currently > excepted from that. Yes, this has been discussed at length in the threads mentioned in the "Link" tags. > > I guess your usecase is a little special in that you were already > calling from userspace into the kernel with SWBP before, which is also > not subject to seccomp; and the syscall is essentially an > arch-specific hack to make the SWBP a little faster. Indeed. The uretprobe mechanism wasn't enforced by seccomp before this syscall. This change preserves this. > > If we do this, we should at least ensure that there is absolutely no > way for anything to happen in sys_uretprobe when no uretprobes are > configured for the process - the first check in the syscall > implementation almost does that, but the implementation could be a bit > stricter. It checks for "regs->ip != trampoline_check_ip()", but if no > uprobe region exists for the process, trampoline_check_ip() returns > `-1 + (uretprobe_syscall_check - uretprobe_trampoline_entry)`. So > there is a userspace instruction pointer 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, and > uprobe_handle_trampoline() will SIGILL us if we get that far without a > valid uretprobe. I'm not sure I understand your point. If creating mappings in that area is prevented, what is the issue? also, this would be related to the uretprobe syscall implementation in general, no? To me this seems unrelated to the seccomp change. Jiri, do you have any input on this? Thanks! Eyal.
On Fri, Feb 7, 2025 at 5:20 PM Eyal Birger <eyal.birger@gmail.com> wrote: > On Fri, Feb 7, 2025 at 7:27 AM Jann Horn <jannh@google.com> wrote: > > > > On Sun, Feb 2, 2025 at 5:29 PM Eyal Birger <eyal.birger@gmail.com> wrote: > > > uretprobe(2) is an performance enhancement system call added to improve > > > uretprobes on x86_64. > > > > > > Confinement environments such as Docker are not aware of this new system > > > call and kill confined processes when uretprobes are attached to them. > > > > FYI, you might have similar issues with Syscall User Dispatch > > (https://docs.kernel.org/admin-guide/syscall-user-dispatch.html) and > > potentially also with ptrace-based sandboxes, depending on what kinda > > processes you inject uprobes into. For Syscall User Dispatch, there is > > already precedent for a bypass based on instruction pointer (see > > syscall_user_dispatch()). > > Thanks. This is interesting. > > Do you know of confinement environments using this? Not for Syscall User Dispatch; I think that was mostly intended for stuff like emulating Windows syscalls in WINE. I'm not sure who actually uses it, I just know a bit about the kernel side of it. From what I know, ptrace sandboxing is a technique used by some configurations of gVisor (https://gvisor.dev/docs/architecture_guide/platforms/#ptrace), though now I see that that page says that this configuration is no longer supported. I am also not sure whether you'd ever have uprobes installed in files from which instructions are executed in this environment. > > > Since uretprobe is a "kernel implementation detail" system call which is > > > not used by userspace application code directly, pass this system call > > > through seccomp without forcing existing userspace confinement environments > > > to be changed. > > > > This makes me feel kinda uncomfortable. The purpose of seccomp() is > > that you can create a process that is as locked down as you want; you > > can use it for some light limits on what a process can do (like in > > Docker), or you can use it to make a process that has access to > > essentially nothing except read(), write() and exit_group(). Even > > stuff like restart_syscall() and rt_sigreturn() is not currently > > excepted from that. > > Yes, this has been discussed at length in the threads mentioned > in the "Link" tags. > > > > > I guess your usecase is a little special in that you were already > > calling from userspace into the kernel with SWBP before, which is also > > not subject to seccomp; and the syscall is essentially an > > arch-specific hack to make the SWBP a little faster. > > Indeed. The uretprobe mechanism wasn't enforced by seccomp before > this syscall. This change preserves this. > > > > > If we do this, we should at least ensure that there is absolutely no > > way for anything to happen in sys_uretprobe when no uretprobes are > > configured for the process - the first check in the syscall > > implementation almost does that, but the implementation could be a bit > > stricter. It checks for "regs->ip != trampoline_check_ip()", but if no > > uprobe region exists for the process, trampoline_check_ip() returns > > `-1 + (uretprobe_syscall_check - uretprobe_trampoline_entry)`. So > > there is a userspace instruction pointer 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, and > > uprobe_handle_trampoline() will SIGILL us if we get that far without a > > valid uretprobe. > > I'm not sure I understand your point. If creating mappings in that > area is prevented, what is the issue? It is usually prevented, not always - root can do it depending on system configuration. Also, in a syscall like this that will be reachable in every sandbox, I think we should try to be more careful about edge cases and avoid things like this offset calculation on address -1. > also, this would be related to the > uretprobe syscall implementation in general, no? Yes. I just think it is relevant to the seccomp change because excepting a syscall from seccomp makes it more important that that syscall is robust and correct. > To me this seems unrelated to the seccomp change. > Jiri, do you have any input on this? > > Thanks! > Eyal.
On Fri, Feb 07, 2025 at 04:27:09PM +0100, Jann Horn wrote: > On Sun, Feb 2, 2025 at 5:29 PM Eyal Birger <eyal.birger@gmail.com> wrote: > > uretprobe(2) is an performance enhancement system call added to improve > > uretprobes on x86_64. > > > > Confinement environments such as Docker are not aware of this new system > > call and kill confined processes when uretprobes are attached to them. > > FYI, you might have similar issues with Syscall User Dispatch > (https://docs.kernel.org/admin-guide/syscall-user-dispatch.html) and > potentially also with ptrace-based sandboxes, depending on what kinda > processes you inject uprobes into. For Syscall User Dispatch, there is > already precedent for a bypass based on instruction pointer (see > syscall_user_dispatch()). > > > Since uretprobe is a "kernel implementation detail" system call which is > > not used by userspace application code directly, pass this system call > > through seccomp without forcing existing userspace confinement environments > > to be changed. > > This makes me feel kinda uncomfortable. The purpose of seccomp() is > that you can create a process that is as locked down as you want; you > can use it for some light limits on what a process can do (like in > Docker), or you can use it to make a process that has access to > essentially nothing except read(), write() and exit_group(). Even > stuff like restart_syscall() and rt_sigreturn() is not currently > excepted from that. > > I guess your usecase is a little special in that you were already > calling from userspace into the kernel with SWBP before, which is also > not subject to seccomp; and the syscall is essentially an > arch-specific hack to make the SWBP a little faster. > > If we do this, we should at least ensure that there is absolutely no > way for anything to happen in sys_uretprobe when no uretprobes are > configured for the process - the first check in the syscall > implementation almost does that, but the implementation could be a bit > stricter. It checks for "regs->ip != trampoline_check_ip()", but if no > uprobe region exists for the process, trampoline_check_ip() returns > `-1 + (uretprobe_syscall_check - uretprobe_trampoline_entry)`. So > there is a userspace instruction pointer 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, and > uprobe_handle_trampoline() will SIGILL us if we get that far without a > valid uretprobe. nice catch, I think change below should fix that thanks, jirka --- diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c index 0c74a4d4df65..9b8837d8f06e 100644 --- a/arch/x86/kernel/uprobes.c +++ b/arch/x86/kernel/uprobes.c @@ -368,19 +368,21 @@ void *arch_uretprobe_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); } SYSCALL_DEFINE0(uretprobe) { struct pt_regs *regs = task_pt_regs(current); - unsigned long err, ip, sp, r11_cx_ax[3]; + unsigned long err, ip, sp, r11_cx_ax[3], tramp; + + tramp = uprobe_get_trampoline_vaddr(); + if (tramp == -1) + goto sigill; - if (regs->ip != trampoline_check_ip()) + if (regs->ip != trampoline_check_ip(tramp)) goto sigill; err = copy_from_user(r11_cx_ax, (void __user *)regs->sp, sizeof(r11_cx_ax));
On Sat, Feb 08, 2025 at 01:03:55AM +0100, Jiri Olsa wrote: > On Fri, Feb 07, 2025 at 04:27:09PM +0100, Jann Horn wrote: > > On Sun, Feb 2, 2025 at 5:29 PM Eyal Birger <eyal.birger@gmail.com> wrote: > > > uretprobe(2) is an performance enhancement system call added to improve > > > uretprobes on x86_64. > > > > > > Confinement environments such as Docker are not aware of this new system > > > call and kill confined processes when uretprobes are attached to them. > > > > FYI, you might have similar issues with Syscall User Dispatch > > (https://docs.kernel.org/admin-guide/syscall-user-dispatch.html) and > > potentially also with ptrace-based sandboxes, depending on what kinda > > processes you inject uprobes into. For Syscall User Dispatch, there is > > already precedent for a bypass based on instruction pointer (see > > syscall_user_dispatch()). > > > > > Since uretprobe is a "kernel implementation detail" system call which is > > > not used by userspace application code directly, pass this system call > > > through seccomp without forcing existing userspace confinement environments > > > to be changed. > > > > This makes me feel kinda uncomfortable. The purpose of seccomp() is > > that you can create a process that is as locked down as you want; you > > can use it for some light limits on what a process can do (like in > > Docker), or you can use it to make a process that has access to > > essentially nothing except read(), write() and exit_group(). Even > > stuff like restart_syscall() and rt_sigreturn() is not currently > > excepted from that. > > > > I guess your usecase is a little special in that you were already > > calling from userspace into the kernel with SWBP before, which is also > > not subject to seccomp; and the syscall is essentially an > > arch-specific hack to make the SWBP a little faster. > > > > If we do this, we should at least ensure that there is absolutely no > > way for anything to happen in sys_uretprobe when no uretprobes are > > configured for the process - the first check in the syscall > > implementation almost does that, but the implementation could be a bit > > stricter. It checks for "regs->ip != trampoline_check_ip()", but if no > > uprobe region exists for the process, trampoline_check_ip() returns > > `-1 + (uretprobe_syscall_check - uretprobe_trampoline_entry)`. So > > there is a userspace instruction pointer 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, and > > uprobe_handle_trampoline() will SIGILL us if we get that far without a > > valid uretprobe. > > nice catch, I think change below should fix that Thanks! Please backport this to -stable too. :) -Kees > > thanks, > jirka > > > --- > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c > index 0c74a4d4df65..9b8837d8f06e 100644 > --- a/arch/x86/kernel/uprobes.c > +++ b/arch/x86/kernel/uprobes.c > @@ -368,19 +368,21 @@ void *arch_uretprobe_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); > } > > SYSCALL_DEFINE0(uretprobe) > { > struct pt_regs *regs = task_pt_regs(current); > - unsigned long err, ip, sp, r11_cx_ax[3]; > + unsigned long err, ip, sp, r11_cx_ax[3], tramp; > + > + tramp = uprobe_get_trampoline_vaddr(); > + if (tramp == -1) > + goto sigill; > > - if (regs->ip != trampoline_check_ip()) > + if (regs->ip != trampoline_check_ip(tramp)) > goto sigill; > > err = copy_from_user(r11_cx_ax, (void __user *)regs->sp, sizeof(r11_cx_ax));
uretprobe(2) is an performance enhancement system call added to improve uretprobes on x86_64. Confinement environments such as Docker are not aware of this new system call and kill confined processes when uretprobes are attached to them. Since uretprobe is a "kernel implementation detail" system call which is not used by userspace application code directly, pass this system call through seccomp without forcing existing userspace confinement environments to be changed. To: Kees Cook <kees@kernel.org> To: Andy Lutomirski <luto@amacapital.net> To: Will Drewry <wad@chromium.org> To: Oleg Nesterov <oleg@redhat.com> To: Masami Hiramatsu (Google) <mhiramat@kernel.org> To: Jiri Olsa <jolsa@kernel.org> To: Andrii Nakryiko <andrii@kernel.org> Cc: linux-kernel@vger.kernel.org Signed-off-by: Eyal Birger <eyal.birger@gmail.com> Eyal Birger (2): seccomp: passthrough uretprobe systemcall without filtering selftests/seccomp: validate uretprobe syscall passes through seccomp kernel/seccomp.c | 24 ++- tools/testing/selftests/seccomp/seccomp_bpf.c | 195 ++++++++++++++++++ 2 files changed, 216 insertions(+), 3 deletions(-)