On Thu, Oct 25, 2018 at 12:13 PM, Oleg Nesterov <o...@redhat.com> wrote: > So again, suppose that "child" is already dead. Its task_struct can't be > freed, > but child->real_parent can point to the already freed memory.
I can't find a path for "child" to be released. I see task_lock() always called on it before it ends up in Yama. (Well, I can't find the lock for switch_mm(), but I assume that's safe because it's switching to the task.) > This means that the 1st walker = rcu_dereference(walker->real_parent) is fine, > this simply reads the child->real_parent pointer, but on the second iteration > > walker = rcu_dereference(walker->real_parent); > > reads the alredy freed memory. What does rcu_read_lock() protect actually protect here? I thought none of the task structs would be freed until after all rcu_read_unlock() finished. > OK. Lets ignore ptracer_exception_found() for the moment. Why do you think the > patch below can't help? > > Oleg. > > --- x/security/yama/yama_lsm.c > +++ x/security/yama/yama_lsm.c > @@ -368,7 +368,8 @@ static int yama_ptrace_access_check(stru > break; > case YAMA_SCOPE_RELATIONAL: > rcu_read_lock(); > - if (!task_is_descendant(current, child) && > + if (!pid_alive(child) || > + !task_is_descendant(current, child) && > !ptracer_exception_found(current, child) && > !ns_capable(__task_cred(child)->user_ns, > CAP_SYS_PTRACE)) > rc = -EPERM; > Hm, documentation there says: * If pid_alive fails, then pointers within the task structure * can be stale and must not be dereferenced. What is the safe pattern for checking vs rcu? -Kees -- Kees Cook