linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@osdl.org>
To: Andrey Borzenkov <arvidjaar@mail.ru>
Cc: linux-kernel@vger.kernel.org, devfs@oss.sgi.com
Subject: Re: [PATCH][2.5.73] stack corruption in devfs_lookup
Date: Sun, 6 Jul 2003 12:03:15 -0700	[thread overview]
Message-ID: <20030706120315.261732bb.akpm@osdl.org> (raw)
In-Reply-To: <200307062058.40797.arvidjaar@mail.ru>

Andrey Borzenkov <arvidjaar@mail.ru> wrote:
>
> When devfs_lookup needs to call devfsd it arranges for other lookups for the 
>  same name to wait. It is using local variable as wait queue head. After 
>  devfsd returns devfs_lookup wakes up all waiters and returns. Unfortunately 
>  there is no garantee all waiters will actually get chance to run and clean up 
>  before devfs_lookup returns. so some of them attempt to access already freed 
>  storage on stack.

OK, but I think there is a simpler fix.  We can rely on the side-effects of
prepare_to_wait() and finish_wait().

The wakeup() will remove all wait_queue_t's from the wait_queue_head, and
so when the waiters wake up and call finish_wait(), they will never touch
the now-out-of-scope waitqueue head.

It is a little faster than the currentcode, too.

Could you please test this?



diff -puN fs/devfs/base.c~devfs-oops-fix-2 fs/devfs/base.c
--- 25/fs/devfs/base.c~devfs-oops-fix-2	2003-07-06 11:55:38.000000000 -0700
+++ 25-akpm/fs/devfs/base.c	2003-07-06 11:59:23.000000000 -0700
@@ -2218,7 +2218,6 @@ static int devfs_d_revalidate_wait (stru
     struct fs_info *fs_info = dir->i_sb->s_fs_info;
     devfs_handle_t parent = get_devfs_entry_from_vfs_inode (dir);
     struct devfs_lookup_struct *lookup_info = dentry->d_fsdata;
-    DECLARE_WAITQUEUE (wait, current);
 
     if ( is_devfsd_or_child (fs_info) )
     {
@@ -2252,11 +2251,12 @@ static int devfs_d_revalidate_wait (stru
     read_lock (&parent->u.dir.lock);
     if (dentry->d_fsdata)
     {
-	set_current_state (TASK_UNINTERRUPTIBLE);
-	add_wait_queue (&lookup_info->wait_queue, &wait);
-	read_unlock (&parent->u.dir.lock);
-	schedule ();
-	remove_wait_queue (&lookup_info->wait_queue, &wait);
+	DEFINE_WAIT(wait);
+
+	prepare_to_wait(&lookup_info->wait_queue, &wait, TASK_UNINTERRUPTIBLE);
+	read_unlock(&parent->u.dir.lock);
+	schedule();
+	finish_wait(&lookup_info->wait_queue, &wait);
     }
     else read_unlock (&parent->u.dir.lock);
     return 1;
@@ -2336,6 +2336,12 @@ out:
     dentry->d_op = &devfs_dops;
     dentry->d_fsdata = NULL;
     write_lock (&parent->u.dir.lock);
+    /*
+     * This wakeup will remove all waiters' wait_queue_t's from the waitqueue
+     * head, because the waiters use prepare_to_wait()/finished_wait().
+     * Hence it is OK that the waitqueue_head goes out of scope immediately
+     * after the wakeup is delivered
+     */
     wake_up (&lookup_info.wait_queue);
     write_unlock (&parent->u.dir.lock);
     devfs_put (de);

_


  reply	other threads:[~2003-07-06 18:47 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <E198K0q-000Am8-00.arvidjaar-mail-ru@f23.mail.ru>
     [not found] ` <Pine.LNX.4.55.0304231157560.1309@marabou.research.att.com>
     [not found]   ` <Pine.LNX.4.55.0305050005230.1278@marabou.research.att.com>
2003-07-06 17:06     ` Andrey Borzenkov
2003-07-06 19:03       ` Andrew Morton [this message]
     [not found]         ` <20030706175405.518f680d.akpm@osdl.org>
2003-07-07 19:06           ` [PATCH][2.5.74] devfs lookup deadlock/stack corruption combined patch Andrey Borzenkov
2003-07-07 21:00             ` Andrew Morton
2003-07-08 17:49               ` Andrey Borzenkov
2003-07-09  1:20                 ` Herbert Poetzl
2003-07-09  1:26                   ` Andrew Morton
2003-07-09  2:09                     ` Herbert Poetzl
2003-07-09 10:34                       ` Thierry Vignaud
2003-07-07 21:41             ` Pavel Roskin
2003-07-26 14:58         ` [PATCH][2.6.0-test1] redesign - stack corruption in devfs_lookup Andrey Borzenkov

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=20030706120315.261732bb.akpm@osdl.org \
    --to=akpm@osdl.org \
    --cc=arvidjaar@mail.ru \
    --cc=devfs@oss.sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH][2.5.73] stack corruption in devfs_lookup' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).