From 07db95a2672505ebf17b08a663ea2266b1b57b2d Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Thu, 12 Mar 2020 13:46:46 +0000 Subject: [PATCH] Bug 1621686 - Fix socket process sandbox's handling of prctl to prevent crash on kernels before 3.17. r=gcp,mjf The special handling of PR_SET_NO_NEW_PRIVS can't be overridden with Allow(); otherwise every thread in the process will repeatedly apply copies of the policy to itself until it reaches whatever limits the kernel imposes, and then we crash so we don't continue execution seemingly unsandboxed. (See also bug 1257361.) The prctl policy for the socket process is still allow-all after this patch; it just prevents crashing the socket process on startup on kernels before 3.17 (which don't support applying the policy atomically to all threads). This patch also adds a comment to try to document this failure mode. Differential Revision: https://phabricator.services.mozilla.com/D66523 --HG-- extra : moz-landing-system : lando --- security/sandbox/linux/SandboxFilter.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/security/sandbox/linux/SandboxFilter.cpp b/security/sandbox/linux/SandboxFilter.cpp index f188281a5493..80b5005c0525 100644 --- a/security/sandbox/linux/SandboxFilter.cpp +++ b/security/sandbox/linux/SandboxFilter.cpp @@ -603,6 +603,11 @@ class SandboxPolicyCommon : public SandboxPolicyBase { // prctl case __NR_prctl: { + // WARNING: do not handle __NR_prctl directly in subclasses; + // override PrctlPolicy instead. The special handling of + // PR_SET_NO_NEW_PRIVS is used to detect that a thread already + // has the policy applied; see also bug 1257361. + if (SandboxInfo::Get().Test(SandboxInfo::kHasSeccompTSync)) { return PrctlPolicy(); } @@ -1536,14 +1541,16 @@ class SocketProcessSandboxPolicy final : public SandboxPolicyCommon { } } + ResultExpr PrctlPolicy() const override { + // FIXME: bug 1619661 + return Allow(); + } + ResultExpr EvaluateSyscall(int sysno) const override { switch (sysno) { case __NR_getrusage: return Allow(); - case __NR_prctl: - return Allow(); - case __NR_ioctl: { static const unsigned long kTypeMask = _IOC_TYPEMASK << _IOC_TYPESHIFT; static const unsigned long kTtyIoctls = TIOCSTI & kTypeMask;