Re: [PATCH v3 seccomp 2/5] seccomp/cache: Add "emulator" to check if filter is constant allow

Kees Cook
2020-10-09T05:41:21Z

On Thu, Oct 08, 2020 at 11:47:17PM -0500, YiFei Zhu wrote:
> On Wed, Sep 30, 2020 at 10:20 AM YiFei Zhu <zhuyifei1999@gmail.com> wrote:
> > @@ -544,7 +577,8 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
> > {
> > struct seccomp_filter *sfilter;
> > int ret;
> > - const bool save_orig = IS_ENABLED(CONFIG_CHECKPOINT_RESTORE);
> > + const bool save_orig = IS_ENABLED(CONFIG_CHECKPOINT_RESTORE) ||
> > + IS_ENABLED(CONFIG_SECCOMP_CACHE_NR_ONLY);
> >
> > if (fprog->len == 0 || fprog->len > BPF_MAXINSNS)
> > return ERR_PTR(-EINVAL);
>
> I'm trying to use __is_defined(SECCOMP_ARCH_NATIVE) here, and got this message:
>
> kernel/seccomp.c: In function ‘seccomp_prepare_filter’:
> ././include/linux/kconfig.h:44:44: error: pasting "__ARG_PLACEHOLDER_"
> and "(" does not give a valid preprocessing token
> 44 | #define ___is_defined(val) ____is_defined(__ARG_PLACEHOLDER_##val)
> | ^~~~~~~~~~~~~~~~~~
> ././include/linux/kconfig.h:43:27: note: in expansion of macro ‘___is_defined’
> 43 | #define __is_defined(x) ___is_defined(x)
> | ^~~~~~~~~~~~~
> kernel/seccomp.c:629:11: note: in expansion of macro ‘__is_defined’
> 629 | __is_defined(SECCOMP_ARCH_NATIVE);
> | ^~~~~~~~~~~~
>
> Looking at the implementation of __is_defined, it is:
>
> #define __ARG_PLACEHOLDER_1 0,
> #define __take_second_arg(__ignored, val, ...) val
> #define __is_defined(x) ___is_defined(x)
> #define ___is_defined(val) ____is_defined(__ARG_PLACEHOLDER_##val)
> #define ____is_defined(arg1_or_junk) __take_second_arg(arg1_or_junk 1, 0)
>
> Hence, when FOO is defined to be 1, then the expansion would be
> __is_defined(FOO) -> ___is_defined(1) ->
> ____is_defined(__ARG_PLACEHOLDER_1) -> __take_second_arg(0, 1, 0) ->
> 1,
> and when FOO is not defined, the expansion would be __is_defined(FOO)
> -> ___is_defined(FOO) -> ____is_defined(__ARG_PLACEHOLDER_FOO) ->
> __take_second_arg(__ARG_PLACEHOLDER_FOO 1, 0) -> 0
>
> However, here SECCOMP_ARCH_NATIVE is an expression from an OR of some
> bits, and __is_defined(SECCOMP_ARCH_NATIVE) would not expand to
> __ARG_PLACEHOLDER_1 during any stage in the preprocessing.
>
> Is there any better way to do this? I'm thinking of just doing #if
> defined(CONFIG_CHECKPOINT_RESTORE) || defined(SECCOMP_ARCH_NATIVE)
> like in Kee's patch.

Yeah, I think that's simplest.

--
Kees Cook