Messages in this thread | | | Date | Wed, 26 Oct 2016 18:10:39 +0200 | From | Oleg Nesterov <> | Subject | Re: hit a KASan bug related to Perf during stress test |
| |
On 10/26, Peter Zijlstra wrote: > > On Tue, Oct 25, 2016 at 04:41:26PM +0200, Oleg Nesterov wrote: > > > > > > So what serialization would close that race? __task_pid_nr_ns() only > > > seems to use RCU nothing more. > > > > I do not see how can we close this race, we obviously do not want to use > > any locking. > > > > That is why I tried to suggest > > > > nr = __task_pid_nr_ns(p, type, event->ns); > > if (!nr && !is_idle_task(p)) > > nr = -1; > > return nr; > > > > but this will report -1 if p runs in another namespace, so perhaps we > > can do > > > > nr = __task_pid_nr_ns(p, type, event->ns); > > if (!nr && p->exit_state) > > // it has already called exit_notify > > nr = -1; > > return nr; > > I think I'm asking how __task_pid_nr_ns() isn't susceptible to this race > ;-)
which race ? ;) it seems that I confused you. Lets ignore the original problem with perf_event_pid()->task_tgid_nr_ns() which can access the freed memory. Lets suppose it is already fixed.
Another problem, as you noted, is that task_tgid_nr_ns/task_pid_nr_ns returns zero if the task exits and this zero can be confused with the swapper's pid.
return pid_alive(p) ? task_pid_nr_ns(p, event->ns) : ~0
still can return zero because pid_alive(p) == T is not stable if we can race with the exiting task, so it can't guarantee that task_pid_nr_ns() won't return 0.
So we can check ->exit_state or, even better, that same pid_alive() after task_pid_nr_ns() returns 0.
nr = task_pid_nr_ns(p); /* avoid -1 if it is idle thread or runs in another ns */ if (!nr && !pid_alive(p)) nr = -1; return nr;
Or I misunderstood you?
Oleg.
| |