On Thu, May 30, 2019 at 2:35 PM Oleg Nesterov <o...@redhat.com> wrote: > On 05/29, Jann Horn wrote: > > --- a/kernel/cred.c > > +++ b/kernel/cred.c > > @@ -450,6 +450,15 @@ int commit_creds(struct cred *new) > > if (task->mm) > > set_dumpable(task->mm, suid_dumpable); > > task->pdeath_signal = 0; > > + /* > > + * If a task drops privileges and becomes nondumpable, > > + * the dumpability change must become visible before > > + * the credential change; otherwise, a __ptrace_may_access() > > + * racing with this change may be able to attach to a task it > > + * shouldn't be able to attach to (as if the task had dropped > > + * privileges without becoming nondumpable). > > + * Pairs with a read barrier in __ptrace_may_access(). > > + */ > > smp_wmb(); > > Hmm. Now that I tried to actually read this patch I do not understand this > wmb(). > > commit_creds() does rcu_assign_pointer(real_cred) which implies > smp_store_release(), > the dumpability change must be visible before ->real_cred is updated without > any > additional barriers?
Oh, yes, I think you're right. So I guess I should make a v2 that still adds the smp_rmb() in __ptrace_may_access(), but gets rid of the smp_wmb() in commit_creds()? (With a comment above the rcu_assign_pointer() that explains the ordering?)
