On Sat, Aug 18, 2007 at 02:22:28AM +0400, Oleg Nesterov wrote:
> Neil Horman wrote:
> >
> > +static int proc_pid_limits(struct task_struct *task, char *buffer)
> > +{
> > + unsigned int i;
> > + int count = 0;
> > + char *bufptr = buffer;
> > +
> > + struct rlimit rlim[RLIM_NLIMITS];
> > +
> > + read_lock(&tasklist_lock);
> > + memcpy(rlim, task->signal->rlim, sizeof(struct rlimit) * RLIM_NLIMITS);
> > + read_unlock(&tasklist_lock);
>
> Please don't re-introduce tasklist_lock unless strictly needed. And in this case
> it doesn't help, sys_getrlimit() changes ->rlim[] under task_lock().
>
> Hovewer, I think the whole patch is not right. The "tsk" itself is pinned, but its
> ->signal is not stable and can be == NULL.
>
> You can use lock_task_sighand() to access ->signal.
>
You're right about the use of task_lock rather than tasklist_lock in getrlimit,
but the comment from lock_task_sighand indicates that its use is predicated on
the prerequisite of locking tasklist_lock, so I think the situation is not that
much of an issue. From what I see the use of lock_task_sighand is used when
modifying values in the signal struct, not when removing it entirely (IIRC it
needs to exist until such time as all sharing processes exit. The fact that we
have an outstanding task struct we are using here guarantees its continued
existence). Since we are only reading signal->rlimit, which is only written to
from sys_setrlimit, we should be safe from corrupted limit reads, which at worst
would cause an erroneous transient data read, rather than any sort of
panic/crash.
Regards
Neil
> Oleg.
--
/***************************************************
*Neil Horman
*Software Engineer
*Red Hat, Inc.
*[email protected]
*gpg keyid: 1024D / 0x92A74FA1
*http://pgp.mit.edu
***************************************************/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
