Re: [PATCH v7 4/9] seccomp: move no_new_privs into seccomp

[Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]

On Tue, Jun 24, 2014 at 12:34 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Tue, Jun 24, 2014 at 12:30 PM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On 06/24, Andy Lutomirski wrote:
>>>
>>> On Tue, Jun 24, 2014 at 12:18 PM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>> >>
>>> >> -struct seccomp { };
>>> >> +struct seccomp {
>>> >> +     unsigned long flags;
>>> >> +};
>>> >
>>> > A bit messy ;)
>>> >
>>> > I am wondering if we can simply do
>>> >
>>> >         static inline bool current_no_new_privs(void)
>>> >         {
>>> >                 if (current->no_new_privs)
>>> >                         return true;
>>> >
>>> >         #ifdef CONFIG_SECCOMP
>>> >                 if (test_thread_flag(TIF_SECCOMP))
>>> >                         return true;
>>> >         #endif
>>>
>>> Nope -- privileged users can enable seccomp w/o nnp.
>>
>> Indeed, I am stupid.
>>
>> Still it would be nice to cleanup this somehow. The new member is only
>> used as a previous ->no_new_privs, just it is long to allow the concurent
>> set/get. Logically it doesn't even belong to seccomp{}.
>
> We could add an unsigned long atomic flags field to task_struct.

I thought that had gotten shot down originally, but given the current
state of the patch series, it would be effectively identical, since my
earlier attempt at keeping sizes the same (with alternate accessors)
was too messy. I will change this as well.

> Grr.  Why isn't there an unsigned *int* atomic bitmask type?  Even u64
> would be better.  unsigned long is useless.

Useless beyond 32 bits. ;)

-Kees

-- 
Kees Cook
Chrome OS Security