public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Todd Inglett <tinglett@vnet.ibm.com>
To: Alexander Viro <viro@math.psu.edu>, linux-kernel@vger.kernel.org
Subject: Re: SMP races in proc with thread_struct
Date: Fri, 04 May 2001 07:34:20 -0500	[thread overview]
Message-ID: <3AF2A1CC.C22A48E7@vnet.ibm.com> (raw)
In-Reply-To: <Pine.GSO.4.21.0105011236140.9771-100000@weyl.math.psu.edu> <3AF14555.8699881B@vnet.ibm.com>

Ok, I've got this isolated.  Here's the sequence of events:

1.  Some process T (probably "top") opens /proc/N/stat.
2.  While holding tasklist_lock the proc code does a get_task_struct()
to add a ref count to the page.
3.  Process N exits.
4.  The parent of process N exits.
5.  Process T reads from the open file.  This calls proc_pid_stat()
which dereferences N's task_struct.  This is ok as Alexander points out
because a reference is held.
6.  Using N's task_struct process T attempt to dereference the *parent*
task struct.  It assumes this is ok because:
	A) it is holding tasklist_lock so N cannot be reparented in a race.
	B) every process *always* has a valid parent.

But this is where hell breaks loose.  Every process has a valid parent
-- unless it is dead and nobody cares.  Process N has already exited and
released from the tasklist while its parent was still alive.  There was
no reason to reparent it.  It just got released.  So N's task_struct has
a dangling ptr to its parent.  Nobody is holding the parent task_struct,
either.  When the parent died memory for its task_struct was released. 
This is ungood.


My opinion here is that this is proc's problem.  When we free a
task_struct it could be "cleaned up" of dangling ptrs, but this is a
hack to cover a bug in proc.

This is not isolated to the parent task_struct, either.  The task_struct
mm is also dereferenced.  It is pretty easy to validate a parent
task_struct ptr (just hold tasklist_lock and run the list to check if it
is still valid -- might not be the *right* task, but it will still be
valid).  However, how do you validate the mm is ok?

It would be nice if there were an easy way to detect when the process
gets into this state.  Certainly it is in this state if the page
reference count is 1.  But multiple processes *could* be reading the
same proc file holding additional artificial ref counts.

Hmm...perhaps if I scan the tasklist for my own task?  If I am not in
the list I either return an error or faked info.  I'll try this....

-- 
-todd

BTW, by adding code to validate the parent my test has now run for 18
hours on a 4-way without failure.  It otherwise would fail within the
first 5 minutes.

  reply	other threads:[~2001-05-04 12:34 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-05-01 14:30 SMP races in proc with thread_struct Todd Inglett
2001-05-01 16:50 ` Alexander Viro
2001-05-03 11:47   ` Todd Inglett
2001-05-04 12:34     ` Todd Inglett [this message]
2001-05-04 12:46       ` Keith Owens
2001-05-04 13:11         ` Andreas Schwab
2001-05-04 13:38           ` Brian Gerst
2001-05-04 23:27           ` Keith Owens
2001-05-04 14:21         ` Andreas Ferber
2001-05-04 15:18           ` Todd Inglett
2001-05-04 16:04       ` Alexander Viro
2001-05-04 17:52       ` [PATCH][RFC] " Alexander Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3AF2A1CC.C22A48E7@vnet.ibm.com \
    --to=tinglett@vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@math.psu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox