[PATCH] D123428: [libunwind] Add configuration to disable sigreturn frame check

Ryan Prichard via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 26 20:58:50 PDT 2022


rprichard added a comment.

In D123428#3475346 <https://reviews.llvm.org/D123428#3475346>, @smeenai wrote:

> In D123428#3444212 <https://reviews.llvm.org/D123428#3444212>, @MaskRay wrote:
>
>> https://github.com/abseil/abseil-cpp/blob/master/absl/debugging/internal/address_is_readable.cc has gone through several iterations. We can use `rt_sigprocmask`.
>
> @rprichard, do you know if this would work for Android? It has the TOCTOU issue, but I imagine it's much simpler than having to manage and synchronize the pipe fd, and we could live with the TOCTOU in practice.

I'm glad @MaskRay found this -- I think it will probably work, and it seems better than assuming the PC is readable.

I see rt_procsigmask listed in bionic/libc/SYSCALLS.TXT, and I don't see it mentioned in any of the bionic/libc/SECCOMP*.txt files. I think that means seccomp is allowing the system call. I looked at kernel/signal.c, and AFAICT it's not doing any security checks that could be a problem. Bionic itself uses `rt_sigprocmask` for (at least) spawning new processes, creating/exiting threads, TLS-related locking, POSIX timers, and abort(). I think any seccomp-like blocking of rt_sigprocmask would have to be very targeted, so I think the syscall is probably allowed everywhere on Android.

It is assuming that the kernel will validate the address before validating the `how`. The kernel has a principle of not breaking userland -- is there a more specific guarantee we can rely on? e.g. The code has this comment:

  // This strategy depends on Linux implementation details,
  // so we rely on the test to alert us if it stops working.

The kernel source is structured as two wrappers around sigprocmask, `SYSCALL_DEFINE4(rt_sigprocmask, ...)` and `COMPAT_SYSCALL_DEFINE4(rt_sigprocmask, ...)`. The wrappers copy user memory to/from kernel memory before calling `sigprocmask`, so it makes sense that they would validate the address first.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123428/new/

https://reviews.llvm.org/D123428



More information about the llvm-commits mailing list