linux-kernel - Re: [PATCH 17/19] sched: Inherit task cookie on fork()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 May 2021 12:22:55 -0400
From:   Chris Hyser <chris.hyser@...cle.com>
To:     Joel Fernandes <joel@...lfernandes.org>,
        Peter Zijlstra <peterz@...radead.org>
Cc:     Josh Don <joshdon@...gle.com>, Ingo Molnar <mingo@...nel.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Valentin Schneider <valentin.schneider@....com>,
        Mel Gorman <mgorman@...e.de>,
        LKML <linux-kernel@...r.kernel.org>,
        Thomas Glexiner <tglx@...utronix.de>
Subject: Re: [PATCH 17/19] sched: Inherit task cookie on fork()

On 5/10/21 12:06 PM, Joel Fernandes wrote:
> Hi Peter,
> 
> On Thu, Apr 22, 2021 at 8:36 AM Peter Zijlstra <peterz@...radead.org> wrote:
>>
>> Note that sched_core_fork() is called from under tasklist_lock, and
>> not from sched_fork() earlier. This avoids a few races later.
>>
>> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
>> ---
>>   include/linux/sched.h     |    2 ++
>>   kernel/fork.c             |    3 +++
>>   kernel/sched/core_sched.c |    6 ++++++
>>   3 files changed, 11 insertions(+)
>>
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -2172,8 +2172,10 @@ const struct cpumask *sched_trace_rd_spa
>>
>>   #ifdef CONFIG_SCHED_CORE
>>   extern void sched_core_free(struct task_struct *tsk);
>> +extern void sched_core_fork(struct task_struct *p);
>>   #else
>>   static inline void sched_core_free(struct task_struct *tsk) { }
>> +static inline void sched_core_fork(struct task_struct *p) { }
>>   #endif
>>
>>   #endif
>> --- a/kernel/fork.c
>> +++ b/kernel/fork.c
>> @@ -2249,6 +2249,8 @@ static __latent_entropy struct task_stru
>>
>>          klp_copy_process(p);
>>
>> +       sched_core_fork(p);
>> +
>>          spin_lock(&current->sighand->siglock);
>>
>>          /*
>> @@ -2336,6 +2338,7 @@ static __latent_entropy struct task_stru
>>          return p;
>>
>>   bad_fork_cancel_cgroup:
>> +       sched_core_free(p);
>>          spin_unlock(&current->sighand->siglock);
>>          write_unlock_irq(&tasklist_lock);
>>          cgroup_cancel_fork(p, args);
>> --- a/kernel/sched/core_sched.c
>> +++ b/kernel/sched/core_sched.c
>> @@ -100,6 +100,12 @@ static unsigned long sched_core_clone_co
>>          return cookie;
>>   }
>>
>> +void sched_core_fork(struct task_struct *p)
>> +{
>> +       RB_CLEAR_NODE(&p->core_node);
>> +       p->core_cookie = sched_core_clone_cookie(current);
> 
> Does this make sense also for !CLONE_THREAD forks?

Yes. Given the absence of a cgroup interface, fork inheritance (clone the cookie) is the best way to create shared 
cookie hierarchies. The security issue you mentioned was handled in my original code by setting a unique cookie on 
'exec', but Peter took that out for the reason mentioned above. It was part of the "lets get this in compromise" effort.

-chrish