Re: [PATCH] seccomp.2: Add note about alarm(2) not being sufficient to limit runtime
Hello Jann, On 03/12/2015 02:07 PM, Jann Horn wrote: > On Wed, Mar 11, 2015 at 10:43:50PM +0100, Mikael Pettersson wrote: >> Jann Horn writes: >> > Or should I throw this patch away and write a patch >> > for the prctl() manpage instead that documents that >> > being able to call sigreturn() implies being able to >> > effectively call sigprocmask(), at least on some >> > architectures like X86? >> >> Well, that is the semantics of sigreturn(). It is essentially >> setcontext() [which includes the actions of sigprocmask()], but >> with restrictions on parameter placement (at least on x86). >> >> You could introduce some setting to restrict that aspect for >> seccomp processes, but you can't change this for normal processes >> without breaking things. > > Then I think it's probably better and easier to just document the existing > behavior? If a new setting would have to be introduced and developers would > need to be aware of that, it's probably easier to just tell everyone to use > SIGKILL. > > Does this manpage patch look good? Patch applied, with Acks from Andy, Mikael, and Kees (I don't usually get patches whose pedigree is that good. Thanks!) I tweaked a few wordings. You can find the changes in Git [1] Cheers, Michael [1] http://git.kernel.org/cgit/docs/man-pages/man-pages.git/commit/?id=65be1b46fb88e14f0af494ac6b53a2d6a63bb860 > --- > man2/seccomp.2 | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/man2/seccomp.2 b/man2/seccomp.2 > index 702ceb8..f762d07 100644 > --- a/man2/seccomp.2 > +++ b/man2/seccomp.2 > @@ -64,6 +64,31 @@ Strict secure computing mode is useful for number-crunching > applications that may need to execute untrusted byte code, perhaps > obtained by reading from a pipe or socket. > > +Note that although the calling thread can no longer call > +.BR sigprocmask (2), > +it can use > +.BR sigreturn (2) > +to block all signals apart from > +.BR SIGKILL > +and > +.BR SIGSTOP . > +Therefore, to reliably terminate it, > +.BR SIGKILL > +has to be used, meaning that e.g. > +.BR alarm (2) > +is not sufficient for restricting its runtime. Instead, use > +.BR timer_create (2) > +with > +.BR SIGEV_SIGNAL > +and > +.BR sigev_signo > +set to > +.BR SIGKILL > +or use > +.BR setrlimit (2) > +to set the hard limit for > +.BR RLIMIT_CPU . > + > This operation is available only if the kernel is configured with > .BR CONFIG_SECCOMP > enabled. > -- Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Linux/UNIX System Programming Training: http://man7.org/training/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] seccomp.2: Add note about alarm(2) not being sufficient to limit runtime
Jann Horn writes: > On Wed, Mar 11, 2015 at 10:43:50PM +0100, Mikael Pettersson wrote: > > Jann Horn writes: > > > Or should I throw this patch away and write a patch > > > for the prctl() manpage instead that documents that > > > being able to call sigreturn() implies being able to > > > effectively call sigprocmask(), at least on some > > > architectures like X86? > > > > Well, that is the semantics of sigreturn(). It is essentially > > setcontext() [which includes the actions of sigprocmask()], but > > with restrictions on parameter placement (at least on x86). > > > > You could introduce some setting to restrict that aspect for > > seccomp processes, but you can't change this for normal processes > > without breaking things. > > Then I think it's probably better and easier to just document the existing > behavior? If a new setting would have to be introduced and developers would > need to be aware of that, it's probably easier to just tell everyone to use > SIGKILL. > > Does this manpage patch look good? LGTM Acked-by: Mikael Pettersson > > --- > man2/seccomp.2 | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/man2/seccomp.2 b/man2/seccomp.2 > index 702ceb8..f762d07 100644 > --- a/man2/seccomp.2 > +++ b/man2/seccomp.2 > @@ -64,6 +64,31 @@ Strict secure computing mode is useful for > number-crunching > applications that may need to execute untrusted byte code, perhaps > obtained by reading from a pipe or socket. > > +Note that although the calling thread can no longer call > +.BR sigprocmask (2), > +it can use > +.BR sigreturn (2) > +to block all signals apart from > +.BR SIGKILL > +and > +.BR SIGSTOP . > +Therefore, to reliably terminate it, > +.BR SIGKILL > +has to be used, meaning that e.g. > +.BR alarm (2) > +is not sufficient for restricting its runtime. Instead, use > +.BR timer_create (2) > +with > +.BR SIGEV_SIGNAL > +and > +.BR sigev_signo > +set to > +.BR SIGKILL > +or use > +.BR setrlimit (2) > +to set the hard limit for > +.BR RLIMIT_CPU . > + > This operation is available only if the kernel is configured with > .BR CONFIG_SECCOMP > enabled. > -- > 2.1.4 -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] seccomp.2: Add note about alarm(2) not being sufficient to limit runtime
On Thu, Mar 12, 2015 at 6:07 AM, Jann Horn wrote: > On Wed, Mar 11, 2015 at 10:43:50PM +0100, Mikael Pettersson wrote: >> Jann Horn writes: >> > Or should I throw this patch away and write a patch >> > for the prctl() manpage instead that documents that >> > being able to call sigreturn() implies being able to >> > effectively call sigprocmask(), at least on some >> > architectures like X86? >> >> Well, that is the semantics of sigreturn(). It is essentially >> setcontext() [which includes the actions of sigprocmask()], but >> with restrictions on parameter placement (at least on x86). >> >> You could introduce some setting to restrict that aspect for >> seccomp processes, but you can't change this for normal processes >> without breaking things. > > Then I think it's probably better and easier to just document the existing > behavior? If a new setting would have to be introduced and developers would > need to be aware of that, it's probably easier to just tell everyone to use > SIGKILL. > > Does this manpage patch look good? > > --- > man2/seccomp.2 | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/man2/seccomp.2 b/man2/seccomp.2 > index 702ceb8..f762d07 100644 > --- a/man2/seccomp.2 > +++ b/man2/seccomp.2 > @@ -64,6 +64,31 @@ Strict secure computing mode is useful for number-crunching > applications that may need to execute untrusted byte code, perhaps > obtained by reading from a pipe or socket. > > +Note that although the calling thread can no longer call > +.BR sigprocmask (2), > +it can use > +.BR sigreturn (2) > +to block all signals apart from > +.BR SIGKILL > +and > +.BR SIGSTOP . > +Therefore, to reliably terminate it, > +.BR SIGKILL > +has to be used, meaning that e.g. > +.BR alarm (2) > +is not sufficient for restricting its runtime. Instead, use > +.BR timer_create (2) > +with > +.BR SIGEV_SIGNAL > +and > +.BR sigev_signo > +set to > +.BR SIGKILL > +or use > +.BR setrlimit (2) > +to set the hard limit for > +.BR RLIMIT_CPU . > + > This operation is available only if the kernel is configured with > .BR CONFIG_SECCOMP > enabled. > -- > 2.1.4 > Thanks! This looks good. Acked-by: Kees Cook -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] seccomp.2: Add note about alarm(2) not being sufficient to limit runtime
On Thu, Mar 12, 2015 at 6:07 AM, Jann Horn wrote: > On Wed, Mar 11, 2015 at 10:43:50PM +0100, Mikael Pettersson wrote: >> Jann Horn writes: >> > Or should I throw this patch away and write a patch >> > for the prctl() manpage instead that documents that >> > being able to call sigreturn() implies being able to >> > effectively call sigprocmask(), at least on some >> > architectures like X86? >> >> Well, that is the semantics of sigreturn(). It is essentially >> setcontext() [which includes the actions of sigprocmask()], but >> with restrictions on parameter placement (at least on x86). >> >> You could introduce some setting to restrict that aspect for >> seccomp processes, but you can't change this for normal processes >> without breaking things. > > Then I think it's probably better and easier to just document the existing > behavior? If a new setting would have to be introduced and developers would > need to be aware of that, it's probably easier to just tell everyone to use > SIGKILL. > > Does this manpage patch look good? Looks good to me. FWIW, if we wanted to fix this in the kernel, I think it could be easier to add SIG_KILL which would be just like SIG_DFL except always fatal even if masked rather than coming up with complicated changes to sigreturn. --Andy > > --- > man2/seccomp.2 | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/man2/seccomp.2 b/man2/seccomp.2 > index 702ceb8..f762d07 100644 > --- a/man2/seccomp.2 > +++ b/man2/seccomp.2 > @@ -64,6 +64,31 @@ Strict secure computing mode is useful for number-crunching > applications that may need to execute untrusted byte code, perhaps > obtained by reading from a pipe or socket. > > +Note that although the calling thread can no longer call > +.BR sigprocmask (2), > +it can use > +.BR sigreturn (2) > +to block all signals apart from > +.BR SIGKILL > +and > +.BR SIGSTOP . > +Therefore, to reliably terminate it, > +.BR SIGKILL > +has to be used, meaning that e.g. > +.BR alarm (2) > +is not sufficient for restricting its runtime. Instead, use > +.BR timer_create (2) > +with > +.BR SIGEV_SIGNAL > +and > +.BR sigev_signo > +set to > +.BR SIGKILL > +or use > +.BR setrlimit (2) > +to set the hard limit for > +.BR RLIMIT_CPU . > + > This operation is available only if the kernel is configured with > .BR CONFIG_SECCOMP > enabled. > -- > 2.1.4 > -- Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] seccomp.2: Add note about alarm(2) not being sufficient to limit runtime
On Wed, Mar 11, 2015 at 10:43:50PM +0100, Mikael Pettersson wrote: > Jann Horn writes: > > Or should I throw this patch away and write a patch > > for the prctl() manpage instead that documents that > > being able to call sigreturn() implies being able to > > effectively call sigprocmask(), at least on some > > architectures like X86? > > Well, that is the semantics of sigreturn(). It is essentially > setcontext() [which includes the actions of sigprocmask()], but > with restrictions on parameter placement (at least on x86). > > You could introduce some setting to restrict that aspect for > seccomp processes, but you can't change this for normal processes > without breaking things. Then I think it's probably better and easier to just document the existing behavior? If a new setting would have to be introduced and developers would need to be aware of that, it's probably easier to just tell everyone to use SIGKILL. Does this manpage patch look good? --- man2/seccomp.2 | 25 + 1 file changed, 25 insertions(+) diff --git a/man2/seccomp.2 b/man2/seccomp.2 index 702ceb8..f762d07 100644 --- a/man2/seccomp.2 +++ b/man2/seccomp.2 @@ -64,6 +64,31 @@ Strict secure computing mode is useful for number-crunching applications that may need to execute untrusted byte code, perhaps obtained by reading from a pipe or socket. +Note that although the calling thread can no longer call +.BR sigprocmask (2), +it can use +.BR sigreturn (2) +to block all signals apart from +.BR SIGKILL +and +.BR SIGSTOP . +Therefore, to reliably terminate it, +.BR SIGKILL +has to be used, meaning that e.g. +.BR alarm (2) +is not sufficient for restricting its runtime. Instead, use +.BR timer_create (2) +with +.BR SIGEV_SIGNAL +and +.BR sigev_signo +set to +.BR SIGKILL +or use +.BR setrlimit (2) +to set the hard limit for +.BR RLIMIT_CPU . + This operation is available only if the kernel is configured with .BR CONFIG_SECCOMP enabled. -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
