Messages in this thread | | | Date | Wed, 4 May 2016 19:39:11 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH] rlimit: locking tidy ups |
| |
On 05/04, Eric W. Biederman wrote: > > Cc'd Oleg as he tends to be deeply involved with this class of locking. > > Mateusz Guzik <mguzik@redhat.com> writes: > > > proc_pid_limits takes ->sighand lock prior to accessing rlimits, but it > > serves no purpose as it does not prevent modifications.
Well. I agree this all needs cleanups or at least additional comments, but
> > @@ -618,14 +618,12 @@ static int proc_pid_limits(struct seq_file *m, struct pid_namespace *ns, > > struct pid *pid, struct task_struct *task) > > { > > unsigned int i; > > - unsigned long flags; > > > > struct rlimit rlim[RLIM_NLIMITS]; > > > > - if (!lock_task_sighand(task, &flags)) > > - return 0; > > + task_lock(task->group_leader);
This is already unsafe. ->group_leader can point to nowhere if this threads exits. lock_task_sighand() ensures that this can't happen.
> > - /* protect tsk->signal and tsk->sighand from disappearing */ > > - read_lock(&tasklist_lock); > > - if (!tsk->sighand) { > > - retval = -ESRCH; > > - goto out; > > + task_lock(tsk->group_leader);
The same, but yes the comment is misleading.
Oleg.
| |