linux-kernel - Re: [PATCH] cred: Use RCU primitives to access RCU pointers

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]

Message-ID: <20200128170426.GA10277@workstation-portable>
Date:   Tue, 28 Jan 2020 22:34:26 +0530
From:   Amol Grover <frextrite@...il.com>
To:     Jann Horn <jannh@...gle.com>
Cc:     David Howells <dhowells@...hat.com>,
        Shakeel Butt <shakeelb@...gle.com>,
        James Morris <jamorris@...ux.microsoft.com>,
        Oleg Nesterov <oleg@...hat.com>,
        Kees Cook <keescook@...omium.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        kernel list <linux-kernel@...r.kernel.org>,
        linux-kernel-mentees@...ts.linuxfoundation.org,
        Joel Fernandes <joel@...lfernandes.org>,
        Madhuparna Bhowmik <madhuparnabhowmik04@...il.com>,
        "Paul E . McKenney" <paulmck@...nel.org>
Subject: Re: [PATCH] cred: Use RCU primitives to access RCU pointers

On Tue, Jan 28, 2020 at 10:30:19AM +0100, Jann Horn wrote:
> On Tue, Jan 28, 2020 at 8:28 AM Amol Grover <frextrite@...il.com> wrote:
> > task_struct.cred and task_struct.real_cred are annotated by __rcu,
> 
> task_struct.cred doesn't actually have RCU semantics though, see
> commit d7852fbd0f0423937fa287a598bfde188bb68c22. For task_struct.cred,
> it would probably be more correct to remove the __rcu annotation?
> 

Hi Jann,

I went through the commit you mentioned. If I understand it correctly,
->cred was not being accessed concurrently (via RCU), hence, a non_rcu
flag was introduced, which determined if the clean-up should wait for
RCU grace-periods or not. And since, the changes were 'thread local'
there was no need to wait for an entire RCU GP to elapse.

The commit too, as you said, mentions the removal of __rcu annotation.
However, simply removing the annotation won't work, as there are quite a
few instances where RCU primitives are used. Even get_current_cred()
uses RCU APIs to get a reference to ->cred. So, currently, maybe we
should continue to use RCU APIs and leave the __rcu annotation in?
(Until someone who takes it on himself to remove __rcu annotation and
fix all the instances). Does that sound good? Or do you want me to
remove __rcu annotation and get the process started?

Thanks
Amol

> > hence use rcu_access_pointer to access them.