Messages in this thread | | | Date | Tue, 1 Sep 2009 18:22:15 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 5/6] core: allow setrlimit to non-current tasks |
| |
Sorry, can't read these series today. Will try tomorrow.
But at first glance some parts looks suspicious to me,
On 08/31, Jiri Slaby wrote: > > @@ -1244,16 +1244,27 @@ int setrlimit(struct task_struct *tsk, unsigned int resource, > > if (new_rlim->rlim_cur > new_rlim->rlim_max) > return -EINVAL; > + > + /* protect tsk->signal and tsk->sighand from disappearing */ > + read_lock_irq(&tasklist_lock);
Why _irq? We can take tasklist_lock for reading without disabling irqs.
And. Unless I misread the patch, update_rlimit_cpu() is called before read_unlock_irq(&tasklist_lock), but update_rlimit_cpu() does spin_unlock_irq(->siglock) and restores interrupts.
> + if (!tsk->signal || !tsk->sighand) {
Please don't check !tsk->signal, !tsk->sighand is enough. If we have ->sighand != NULL (under lock) ->signal must be valid.
But I dislike the fact the patch uses tasklist_lock. Can't lock_task_sighand() work for you? (of course, in this case update_rlimit_cpu() should be updated too).
Once again, I didn't actually read this series yet, sorry.
Oleg.
| |