Re: SMP races in proc with thread_struct

Alexander Viro
2001-05-01T16:51:16Z

On Tue, 1 May 2001, Todd Inglett wrote:

> Perhaps this is old news...but...
>
> I can easily create a race when reading /proc/<pid>/stat
> (fs/proc/{base.c,array.c}) where a rapidly reading application, such as
> "top", starts reading stats for a thread which goes away during the
> read. This is easily reproduced with a program that rapidly forks and
> exits while top is running.
>
> On inspection, I don't see how the code can expect the thread_struct to
> stay around since it is not holding any lock for the duration of its
> use. The code could hold the thread_struct's lock (after verifying it
> still exists while holding tasklist_lock I would imagine), but for
> performance I would think a better solution would be to copy the struct
> since stale data is probably ok in this case.
>
> Dereferencing a non-existent thread_struct is clearly not ok.
>
> Would anyone familiar with this code care to comment?

Code bumps the reference count of couple of pages that hold task_struct
when it opens the file.

Thus when task exits they won't be freed and won't be reused. Checking
that exit had already happened is trivial (and done). And for anything
long said lock is held only while we acquire the reference
to task_struct component.

When you finally release the dentry of /proc/<pid> (and thus are guaranteed
that none of its children are used) you drop the reference to these two pages.
If process had exited while you were holding them - well, that's when
they are freed.

I really wonder what you've managed to reproduce - normal behaviour is
that as soon as task exits you can't access /proc/<pid> and any
access to already opened files in it fails with -ENOENT. Had you been
able to get something else?