2015-06-15 22:25:45

by Kees Cook

[permalink] [raw]
Subject: Re: [kees:seccomp/tip 2/2] kernel/ptrace.c:567:3: note: in expansion of macro 'if'

Looks like dead-code elimination happens after structures are
accessed, so this needs to use the accessor. Sorry I missed this
earlier!

I've fixed it up as:

- if (current->seccomp.mode != SECCOMP_MODE_DISABLED ||
+ if (seccomp_mode(&current->seccomp) != SECCOMP_MODE_DISABLED ||

-Kees


On Mon, Jun 15, 2015 at 3:13 PM, kbuild test robot
<[email protected]> wrote:
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git seccomp/tip
> head: 25234531b68b83b0a5967459613195a2ecc6b5b4
> commit: 25234531b68b83b0a5967459613195a2ecc6b5b4 [2/2] seccomp: add ptrace options for suspend/resume
> config: x86_64-randconfig-x005-201524 (attached as .config)
> reproduce:
> git checkout 25234531b68b83b0a5967459613195a2ecc6b5b4
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All warnings (new ones prefixed by >>):
>
> In file included from include/uapi/linux/stddef.h:1:0,
> from include/linux/stddef.h:4,
> from include/uapi/linux/posix_types.h:4,
> from include/uapi/linux/types.h:13,
> from include/linux/types.h:5,
> from include/uapi/linux/capability.h:16,
> from include/linux/capability.h:15,
> from kernel/ptrace.c:10:
> kernel/ptrace.c: In function 'ptrace_setoptions':
> kernel/ptrace.c:567:23: error: 'struct seccomp' has no member named 'mode'
> if (current->seccomp.mode != SECCOMP_MODE_DISABLED ||
> ^
> include/linux/compiler.h:145:28: note: in definition of macro '__trace_if'
> if (__builtin_constant_p((cond)) ? !!(cond) : \
> ^
>>> kernel/ptrace.c:567:3: note: in expansion of macro 'if'
> if (current->seccomp.mode != SECCOMP_MODE_DISABLED ||
> ^
> kernel/ptrace.c:567:23: error: 'struct seccomp' has no member named 'mode'
> if (current->seccomp.mode != SECCOMP_MODE_DISABLED ||
> ^
> include/linux/compiler.h:145:40: note: in definition of macro '__trace_if'
> if (__builtin_constant_p((cond)) ? !!(cond) : \
> ^
>>> kernel/ptrace.c:567:3: note: in expansion of macro 'if'
> if (current->seccomp.mode != SECCOMP_MODE_DISABLED ||
> ^
> kernel/ptrace.c:567:23: error: 'struct seccomp' has no member named 'mode'
> if (current->seccomp.mode != SECCOMP_MODE_DISABLED ||
> ^
> include/linux/compiler.h:156:16: note: in definition of macro '__trace_if'
> ______r = !!(cond); \
> ^
>>> kernel/ptrace.c:567:3: note: in expansion of macro 'if'
> if (current->seccomp.mode != SECCOMP_MODE_DISABLED ||
> ^
>
> vim +/if +567 kernel/ptrace.c
>
> 551
> 552 static int ptrace_setoptions(struct task_struct *child, unsigned long data)
> 553 {
> 554 unsigned flags;
> 555
> 556 if (data & ~(unsigned long)PTRACE_O_MASK)
> 557 return -EINVAL;
> 558
> 559 if (unlikely(data & PTRACE_O_SUSPEND_SECCOMP)) {
> 560 if (!config_enabled(CONFIG_CHECKPOINT_RESTORE) ||
> 561 !config_enabled(CONFIG_SECCOMP))
> 562 return -EINVAL;
> 563
> 564 if (!capable(CAP_SYS_ADMIN))
> 565 return -EPERM;
> 566
> > 567 if (current->seccomp.mode != SECCOMP_MODE_DISABLED ||
> 568 current->ptrace & PT_SUSPEND_SECCOMP)
> 569 return -EPERM;
> 570 }
> 571
> 572 /* Avoid intermediate state when all opts are cleared */
> 573 flags = child->ptrace;
> 574 flags &= ~(PTRACE_O_MASK << PT_OPT_FLAG_SHIFT);
> 575 flags |= (data << PT_OPT_FLAG_SHIFT);
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> http://lists.01.org/mailman/listinfo/kbuild Intel Corporation



--
Kees Cook
Chrome OS Security


2015-06-15 22:32:10

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [kees:seccomp/tip 2/2] kernel/ptrace.c:567:3: note: in expansion of macro 'if'

On 06/15, Kees Cook wrote:
>
> Looks like dead-code elimination happens after structures are
> accessed, so this needs to use the accessor.

Ah, I forgot we have the dummy "struct seccomp { }" for SECCOMP=n

> Sorry I missed this
> earlier!

Me too :/

Oleg.

2015-06-16 13:26:33

by Tycho Andersen

[permalink] [raw]
Subject: Re: [kees:seccomp/tip 2/2] kernel/ptrace.c:567:3: note: in expansion of macro 'if'

On Mon, Jun 15, 2015 at 03:25:39PM -0700, Kees Cook wrote:
> Looks like dead-code elimination happens after structures are
> accessed, so this needs to use the accessor. Sorry I missed this
> earlier!

Whoops, thanks!

Tycho