On Wed, May 29, 2019 at 07:38:46PM +0200, Jann Horn wrote: > On Wed, May 29, 2019 at 6:21 PM Oleg Nesterov <o...@redhat.com> wrote: > > On 05/29, Jann Horn wrote: > > > (I have no clue whatsoever what the relevant tree for this is, but I > > > guess Oleg is the relevant maintainer?) > > > > we usually route ptrace changes via -mm tree, plus I lost my account on > > korg. > > > > > --- a/kernel/ptrace.c > > > +++ b/kernel/ptrace.c > > > @@ -324,6 +324,16 @@ static int __ptrace_may_access(struct task_struct > > > *task, unsigned int mode) > > > return -EPERM; > > > ok: > > > rcu_read_unlock(); > > > + /* > > > + * If a task drops privileges and becomes nondumpable (through a > > > syscall > > > + * like setresuid()) while we are trying to access it, we must > > > ensure > > > + * that the dumpability is read after the credentials; otherwise, > > > + * we may be able to attach to a task that we shouldn't be able to > > > + * attach to (as if the task had dropped privileges without becoming > > > + * nondumpable). > > > + * Pairs with a write barrier in commit_creds(). > > > + */ > > > + smp_rmb(); > > > > (I am wondering if smp_acquire__after_ctrl_dep() could be used instead, > > just to > > make this code look more confusing) > > Uuh, I had no idea that that barrier type exists. The helper isn't > even explicitly mentioned in Documentation/memory-barriers.rst. I > don't really want to use dark magic in the middle of ptrace access > logic... > > Anyway, looking at it, I think smp_acquire__after_ctrl_dep() doesn't > make sense here; quoting the documentation: "A load-load control > dependency requires a full read memory barrier, not simply a data > dependency barrier to make it work correctly". IIUC > smp_acquire__after_ctrl_dep() is for cases in which you would > otherwise need a full memory barrier - smp_mb() - and you want to be > able to reduce it to a read barrier.
It is supposed to be used when you want an ACQUIRE but you only have a control dependency (so you "augment the dependency" with this barrier). FWIW, I do agree on the "dark magic"..., and I'd strongly recommend to not use this barrier (or, at least, to use it with high suspicion). Andrea
