Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752494AbbFBTaw (ORCPT ); Tue, 2 Jun 2015 15:30:52 -0400 Received: from thejh.net ([37.221.195.125]:50708 "EHLO thejh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751210AbbFBTap (ORCPT ); Tue, 2 Jun 2015 15:30:45 -0400 X-Greylist: delayed 350 seconds by postgrey-1.27 at vger.kernel.org; Tue, 02 Jun 2015 15:30:44 EDT Date: Tue, 2 Jun 2015 21:24:50 +0200 From: Jann Horn To: Pavel Emelyanov Cc: Oleg Nesterov , Tycho Andersen , linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, Kees Cook , Andy Lutomirski , Will Drewry , Roland McGrath , "Serge E. Hallyn" Subject: Re: [PATCH] seccomp: add ptrace commands for suspend/resume Message-ID: <20150602192450.GA14907@pc.thejh.net> References: <1433186918-9626-1-git-send-email-tycho.andersen@canonical.com> <20150602182829.GA23449@redhat.com> <556DFDB2.3050205@parallels.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="GvXjxJ+pjyke8COw" Content-Disposition: inline In-Reply-To: <556DFDB2.3050205@parallels.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2808 Lines: 73 --GvXjxJ+pjyke8COw Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jun 02, 2015 at 10:02:10PM +0300, Pavel Emelyanov wrote: >=20 > >> +int suspend_seccomp(struct task_struct *task) > >> +{ > >> + int ret =3D -EACCES; > >> + > >> + spin_lock_irq(&task->sighand->siglock); > >> + > >> + if (!capable(CAP_SYS_ADMIN)) > >> + goto out; > >=20 > > I am puzzled ;) Why do we need ->siglock? And even if we need it, why > > we can't check CAP_SYS_ADMIN lockless? > >=20 > > And I am not sure I understand why do we need the additional security > > check, but I leave this to you and Andy. > >=20 > > If you have the rights to trace this task, then you can do anything > > the tracee could do without the filtering. >=20 > I think _this_ check is required, otherwise the seccomp-ed task (in > filtered mode) fork-s a child, then this child ptrace-attach to parent > (allowed) then suspend its seccomd. And -- we have unpriviledged process= =20 > de-seccomped. If you can ptrace(), you can already escape from seccomp. See this section in man 2 seccomp, in the SECCOMP_RET_TRACE section: The seccomp check will not be run again after the tracer is notified. (This means that seccomp-based sandboxes must not allow use of ptrace(2)=E2=80=94even of other sandboxed proces= ses=E2=80=94 without extreme care; ptracers can use this mechanism to escape from the seccomp sandbox.) (But I think there have been discussions about changing that behavior in the future?) --GvXjxJ+pjyke8COw Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVbgMCAAoJED4KNFJOeCOoB60P/if9eXOJ6Cg1rRQJanS0+oLE P1a0pS+TChhRMUJBd8dD4UTNbqFG1+Y6ovARpuzzttXh/BIEEPeENVrWYTCes3TZ p57BjT/J0cA22ey4N10TMeUEcc0zZisOHjMf2swQgWGhKgbDxkWoMMjsj8HdUaOL DdTp/5OkfrvPSLulFmn/UCe+Orv00aq4oObLiV8k0OtyH4EiKOd0hYgUjxHcPK8O epJOimMEp1u5kNUEChNxGI713RlsIhWaJyyrPZZZadZ2LxP4h/BBKvtxzhoGlYTW smybdCrYAlXSBhqJZe1Hi3UKUA+t8I2wOIV73u3czkoDwnfUMzIspwFUz58/jCnx DWVFa5X0iTi702+aIua83P/Y2JvAVguKx+NlIv2xcjGDiUjO+hxL3rTUpbGOmqRs 5vloW7KItGbXkEld009P/PBe3/lBsX357ACiomHR8P4LYPwKnDbeeENFB3S/3xOu Z8JAdGe4eV9Lsref7juzjpDw8Jl8xp/NwMaGidBWEfm3olZ0jWf8rkpcrhKmNF0c casoz3kzyQ3GxY61KSA5D5TIqshIF4nQc1tNpYCKtL5ik+Vic6FZmHWVCXlTPHxl toBHjHwjmG/FYFOK93ygYtBtoCBoc4aoF/MfGMV+zWSQjWfePgEp9bB1nDqISCAl LN6veSN9Xha5vuydmBl1 =J3kN -----END PGP SIGNATURE----- --GvXjxJ+pjyke8COw-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/