diff mbox series

[v3,seccomp,1/5] x86: Enable seccomp architecture tracking

Message ID 484392624b475cc25d90a787525ede70df9f7d51.1601478774.git.yifeifz2@illinois.edu (mailing list archive)
State Not Applicable
Headers show
Series seccomp: Add bitmap cache of constant allow filter results | expand

Commit Message

YiFei Zhu Sept. 30, 2020, 3:19 p.m. UTC 
From: Kees Cook <keescook@chromium.org>

Provide seccomp internals with the details to calculate which syscall
table the running kernel is expecting to deal with. This allows for
efficient architecture pinning and paves the way for constant-action
bitmaps.

Signed-off-by: Kees Cook <keescook@chromium.org>
[YiFei: Removed x32, added macro for nr_syscalls]
Signed-off-by: YiFei Zhu <yifeifz2@illinois.edu>
---
 arch/x86/include/asm/seccomp.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Kees Cook Sept. 30, 2020, 9:21 p.m. UTC | #1 
On Wed, Sep 30, 2020 at 10:19:12AM -0500, YiFei Zhu wrote:
> From: Kees Cook <keescook@chromium.org>
> 
> Provide seccomp internals with the details to calculate which syscall
> table the running kernel is expecting to deal with. This allows for
> efficient architecture pinning and paves the way for constant-action
> bitmaps.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> [YiFei: Removed x32, added macro for nr_syscalls]
> Signed-off-by: YiFei Zhu <yifeifz2@illinois.edu>
> ---
>  arch/x86/include/asm/seccomp.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/x86/include/asm/seccomp.h b/arch/x86/include/asm/seccomp.h
> index 2bd1338de236..7b3a58271656 100644
> --- a/arch/x86/include/asm/seccomp.h
> +++ b/arch/x86/include/asm/seccomp.h
> @@ -16,6 +16,18 @@
>  #define __NR_seccomp_sigreturn_32	__NR_ia32_sigreturn
>  #endif
>  
> +#ifdef CONFIG_X86_64
> +# define SECCOMP_ARCH_DEFAULT			AUDIT_ARCH_X86_64
> +# define SECCOMP_ARCH_DEFAULT_NR		NR_syscalls

bikeshedding: let's call these SECCOMP_ARCH_NATIVE* -- I think it's more
descriptive.

> +# ifdef CONFIG_COMPAT
> +#  define SECCOMP_ARCH_COMPAT			AUDIT_ARCH_I386
> +#  define SECCOMP_ARCH_COMPAT_NR		IA32_NR_syscalls
> +# endif
> +#else /* !CONFIG_X86_64 */
> +# define SECCOMP_ARCH_DEFAULT		AUDIT_ARCH_I386
> +# define SECCOMP_ARCH_DEFAULT_NR	NR_syscalls
> +#endif
> +
>  #include <asm-generic/seccomp.h>
>  
>  #endif /* _ASM_X86_SECCOMP_H */
> -- 
> 2.28.0
> 

But otherwise, yes, looks good to me. For this patch, I think the S-o-b chain is probably more
accurately captured as:

Signed-off-by: Kees Cook <keescook@chromium.org>
Co-developed-by: YiFei Zhu <yifeifz2@illinois.edu>
Signed-off-by: YiFei Zhu <yifeifz2@illinois.edu>
Jann Horn Sept. 30, 2020, 9:33 p.m. UTC | #2 
On Wed, Sep 30, 2020 at 11:21 PM Kees Cook <keescook@chromium.org> wrote:
> On Wed, Sep 30, 2020 at 10:19:12AM -0500, YiFei Zhu wrote:
> > From: Kees Cook <keescook@chromium.org>
> >
> > Provide seccomp internals with the details to calculate which syscall
> > table the running kernel is expecting to deal with. This allows for
> > efficient architecture pinning and paves the way for constant-action
> > bitmaps.
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > [YiFei: Removed x32, added macro for nr_syscalls]
> > Signed-off-by: YiFei Zhu <yifeifz2@illinois.edu>
[...]
> But otherwise, yes, looks good to me. For this patch, I think the S-o-b chain is probably more
> accurately captured as:
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Co-developed-by: YiFei Zhu <yifeifz2@illinois.edu>
> Signed-off-by: YiFei Zhu <yifeifz2@illinois.edu>

(Technically, https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
says that "every Co-developed-by: must be immediately followed by a
Signed-off-by: of the associated co-author" (and has an example of how
that should look).)
Kees Cook Sept. 30, 2020, 10:53 p.m. UTC | #3 
On Wed, Sep 30, 2020 at 11:33:15PM +0200, Jann Horn wrote:
> On Wed, Sep 30, 2020 at 11:21 PM Kees Cook <keescook@chromium.org> wrote:
> > On Wed, Sep 30, 2020 at 10:19:12AM -0500, YiFei Zhu wrote:
> > > From: Kees Cook <keescook@chromium.org>
> > >
> > > Provide seccomp internals with the details to calculate which syscall
> > > table the running kernel is expecting to deal with. This allows for
> > > efficient architecture pinning and paves the way for constant-action
> > > bitmaps.
> > >
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > [YiFei: Removed x32, added macro for nr_syscalls]
> > > Signed-off-by: YiFei Zhu <yifeifz2@illinois.edu>
> [...]
> > But otherwise, yes, looks good to me. For this patch, I think the S-o-b chain is probably more
> > accurately captured as:
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Co-developed-by: YiFei Zhu <yifeifz2@illinois.edu>
> > Signed-off-by: YiFei Zhu <yifeifz2@illinois.edu>
> 
> (Technically, https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
> says that "every Co-developed-by: must be immediately followed by a
> Signed-off-by: of the associated co-author" (and has an example of how
> that should look).)

Right, but it is not needed for the commit author (here, the From:),
the second example given in the docs shows this:

	From: From Author <from@author.example.org>

	<changelog>

	Co-developed-by: Random Co-Author <random@coauthor.example.org>
	Signed-off-by: Random Co-Author <random@coauthor.example.org>
	Signed-off-by: From Author <from@author.example.org>
	Co-developed-by: Submitting Co-Author <sub@coauthor.example.org>
	Signed-off-by: Submitting Co-Author <sub@coauthor.example.org>

and there is no third co-developer, so it's:

	From: From Author <from@author.example.org>

	<changelog>

	Signed-off-by: From Author <from@author.example.org>
	Co-developed-by: Submitting Co-Author <sub@coauthor.example.org>
	Signed-off-by: Submitting Co-Author <sub@coauthor.example.org>

If I'm the From, and YiFei Zhu is the submitting co-developer, then
it's:

	From: Kees Cook <keescook@chromium.org>

	<changelog>

	Signed-off-by: Kees Cook <keescook@chromium.org>
	Co-developed-by: YiFei Zhu <yifeifz2@illinois.edu>
	Signed-off-by: YiFei Zhu <yifeifz2@illinois.edu>

which is what I suggested.
Jann Horn Sept. 30, 2020, 11:15 p.m. UTC | #4 
On Thu, Oct 1, 2020 at 12:53 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Wed, Sep 30, 2020 at 11:33:15PM +0200, Jann Horn wrote:
> > On Wed, Sep 30, 2020 at 11:21 PM Kees Cook <keescook@chromium.org> wrote:
> > > On Wed, Sep 30, 2020 at 10:19:12AM -0500, YiFei Zhu wrote:
> > > > From: Kees Cook <keescook@chromium.org>
> > > >
> > > > Provide seccomp internals with the details to calculate which syscall
> > > > table the running kernel is expecting to deal with. This allows for
> > > > efficient architecture pinning and paves the way for constant-action
> > > > bitmaps.
> > > >
> > > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > > [YiFei: Removed x32, added macro for nr_syscalls]
> > > > Signed-off-by: YiFei Zhu <yifeifz2@illinois.edu>
> > [...]
> > > But otherwise, yes, looks good to me. For this patch, I think the S-o-b chain is probably more
> > > accurately captured as:
> > >
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > Co-developed-by: YiFei Zhu <yifeifz2@illinois.edu>
> > > Signed-off-by: YiFei Zhu <yifeifz2@illinois.edu>
> >
> > (Technically, https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
> > says that "every Co-developed-by: must be immediately followed by a
> > Signed-off-by: of the associated co-author" (and has an example of how
> > that should look).)
>
> Right, but it is not needed for the commit author (here, the From:),
> the second example given in the docs shows this:

Aah, right. Nevermind, sorry for the noise.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/seccomp.h b/arch/x86/include/asm/seccomp.h
index 2bd1338de236..7b3a58271656 100644
--- a/arch/x86/include/asm/seccomp.h
+++ b/arch/x86/include/asm/seccomp.h
@@ -16,6 +16,18 @@ 
 #define __NR_seccomp_sigreturn_32	__NR_ia32_sigreturn
 #endif
 
+#ifdef CONFIG_X86_64
+# define SECCOMP_ARCH_DEFAULT			AUDIT_ARCH_X86_64
+# define SECCOMP_ARCH_DEFAULT_NR		NR_syscalls
+# ifdef CONFIG_COMPAT
+#  define SECCOMP_ARCH_COMPAT			AUDIT_ARCH_I386
+#  define SECCOMP_ARCH_COMPAT_NR		IA32_NR_syscalls
+# endif
+#else /* !CONFIG_X86_64 */
+# define SECCOMP_ARCH_DEFAULT		AUDIT_ARCH_I386
+# define SECCOMP_ARCH_DEFAULT_NR	NR_syscalls
+#endif
+
 #include <asm-generic/seccomp.h>
 
 #endif /* _ASM_X86_SECCOMP_H */