Commit 9f49f9f3 authored by Andrew Morton's avatar Andrew Morton Committed by Linus Torvalds

[PATCH] devfs_lookup stack corruption fix rework

From: Andrey Borzenkov <arvidjaar@mail.ru>

A while back Andrey fixed a devfs bug in which we were running
remove_wait_queue() against a wait_queue_head which was on another process's
stack, and which had gone out of scope.

The patch reverts that fix and does it the same way as 2.4: just leave the
waitqueue struct dangling on the waitqueue_head: there is no need to touch it
at all.

It adds a big comment explaining why we are doing this nasty thing.
parent 74b0ab1b
...@@ -2221,46 +2221,8 @@ struct devfs_lookup_struct ...@@ -2221,46 +2221,8 @@ struct devfs_lookup_struct
{ {
devfs_handle_t de; devfs_handle_t de;
wait_queue_head_t wait_queue; wait_queue_head_t wait_queue;
atomic_t count;
}; };
static struct devfs_lookup_struct *
new_devfs_lookup_struct(void)
{
struct devfs_lookup_struct *p = kmalloc(sizeof(*p), GFP_KERNEL);
if (!p)
return NULL;
init_waitqueue_head (&p->wait_queue);
atomic_set(&p->count, 1);
return p;
}
static void
get_devfs_lookup_struct(struct devfs_lookup_struct *info)
{
if (info)
atomic_inc(&info->count);
else {
printk(KERN_ERR "null devfs_lookup_struct pointer\n");
dump_stack();
}
}
static void
put_devfs_lookup_struct(struct devfs_lookup_struct *info)
{
if (info) {
if (!atomic_dec_and_test(&info->count))
return;
kfree(info);
} else {
printk(KERN_ERR "null devfs_lookup_struct pointer\n");
dump_stack();
}
}
/* XXX: this doesn't handle the case where we got a negative dentry /* XXX: this doesn't handle the case where we got a negative dentry
but a devfs entry has been registered in the meanwhile */ but a devfs entry has been registered in the meanwhile */
static int devfs_d_revalidate_wait (struct dentry *dentry, struct nameidata *nd) static int devfs_d_revalidate_wait (struct dentry *dentry, struct nameidata *nd)
...@@ -2303,13 +2265,19 @@ static int devfs_d_revalidate_wait (struct dentry *dentry, struct nameidata *nd) ...@@ -2303,13 +2265,19 @@ static int devfs_d_revalidate_wait (struct dentry *dentry, struct nameidata *nd)
read_lock (&parent->u.dir.lock); read_lock (&parent->u.dir.lock);
if (dentry->d_fsdata) if (dentry->d_fsdata)
{ {
get_devfs_lookup_struct(lookup_info);
set_current_state (TASK_UNINTERRUPTIBLE); set_current_state (TASK_UNINTERRUPTIBLE);
add_wait_queue (&lookup_info->wait_queue, &wait); add_wait_queue (&lookup_info->wait_queue, &wait);
read_unlock (&parent->u.dir.lock); read_unlock (&parent->u.dir.lock);
schedule (); schedule ();
remove_wait_queue (&lookup_info->wait_queue, &wait); /*
put_devfs_lookup_struct(lookup_info); * This does not need nor should remove wait from wait_queue.
* Wait queue head is never reused - nothing is ever added to it
* after all waiters have been waked up and head itself disappears
* very soon after it. Moreover it is local variable on stack that
* is likely to have already disappeared so any reference to it
* at this point is buggy.
*/
} }
else read_unlock (&parent->u.dir.lock); else read_unlock (&parent->u.dir.lock);
return 1; return 1;
...@@ -2321,7 +2289,7 @@ static int devfs_d_revalidate_wait (struct dentry *dentry, struct nameidata *nd) ...@@ -2321,7 +2289,7 @@ static int devfs_d_revalidate_wait (struct dentry *dentry, struct nameidata *nd)
static struct dentry *devfs_lookup (struct inode *dir, struct dentry *dentry, struct nameidata *nd) static struct dentry *devfs_lookup (struct inode *dir, struct dentry *dentry, struct nameidata *nd)
{ {
struct devfs_entry tmp; /* Must stay in scope until devfsd idle again */ struct devfs_entry tmp; /* Must stay in scope until devfsd idle again */
struct devfs_lookup_struct *lookup_info; struct devfs_lookup_struct lookup_info;
struct fs_info *fs_info = dir->i_sb->s_fs_info; struct fs_info *fs_info = dir->i_sb->s_fs_info;
struct devfs_entry *parent, *de; struct devfs_entry *parent, *de;
struct inode *inode; struct inode *inode;
...@@ -2338,10 +2306,9 @@ static struct dentry *devfs_lookup (struct inode *dir, struct dentry *dentry, st ...@@ -2338,10 +2306,9 @@ static struct dentry *devfs_lookup (struct inode *dir, struct dentry *dentry, st
read_lock (&parent->u.dir.lock); read_lock (&parent->u.dir.lock);
de = _devfs_search_dir (parent, dentry->d_name.name, dentry->d_name.len); de = _devfs_search_dir (parent, dentry->d_name.name, dentry->d_name.len);
read_unlock (&parent->u.dir.lock); read_unlock (&parent->u.dir.lock);
lookup_info = new_devfs_lookup_struct(); lookup_info.de = de;
if (!lookup_info) return ERR_PTR(-ENOMEM); init_waitqueue_head (&lookup_info.wait_queue);
lookup_info->de = de; dentry->d_fsdata = &lookup_info;
dentry->d_fsdata = lookup_info;
if (de == NULL) if (de == NULL)
{ /* Try with devfsd. For any kind of failure, leave a negative dentry { /* Try with devfsd. For any kind of failure, leave a negative dentry
so someone else can deal with it (in the case where the sysadmin so someone else can deal with it (in the case where the sysadmin
...@@ -2351,7 +2318,6 @@ static struct dentry *devfs_lookup (struct inode *dir, struct dentry *dentry, st ...@@ -2351,7 +2318,6 @@ static struct dentry *devfs_lookup (struct inode *dir, struct dentry *dentry, st
if (try_modload (parent, fs_info, if (try_modload (parent, fs_info,
dentry->d_name.name, dentry->d_name.len, &tmp) < 0) dentry->d_name.name, dentry->d_name.len, &tmp) < 0)
{ /* Lookup event was not queued to devfsd */ { /* Lookup event was not queued to devfsd */
put_devfs_lookup_struct(lookup_info);
d_add (dentry, NULL); d_add (dentry, NULL);
return NULL; return NULL;
} }
...@@ -2363,7 +2329,7 @@ static struct dentry *devfs_lookup (struct inode *dir, struct dentry *dentry, st ...@@ -2363,7 +2329,7 @@ static struct dentry *devfs_lookup (struct inode *dir, struct dentry *dentry, st
revalidation */ revalidation */
up (&dir->i_sem); up (&dir->i_sem);
wait_for_devfsd_finished (fs_info); /* If I'm not devfsd, must wait */ wait_for_devfsd_finished (fs_info); /* If I'm not devfsd, must wait */
de = lookup_info->de; de = lookup_info.de;
/* If someone else has been so kind as to make the inode, we go home /* If someone else has been so kind as to make the inode, we go home
early */ early */
if (dentry->d_inode) goto out; if (dentry->d_inode) goto out;
...@@ -2390,8 +2356,7 @@ static struct dentry *devfs_lookup (struct inode *dir, struct dentry *dentry, st ...@@ -2390,8 +2356,7 @@ static struct dentry *devfs_lookup (struct inode *dir, struct dentry *dentry, st
write_lock (&parent->u.dir.lock); write_lock (&parent->u.dir.lock);
dentry->d_op = &devfs_dops; dentry->d_op = &devfs_dops;
dentry->d_fsdata = NULL; dentry->d_fsdata = NULL;
wake_up (&lookup_info->wait_queue); wake_up (&lookup_info.wait_queue);
put_devfs_lookup_struct(lookup_info);
write_unlock (&parent->u.dir.lock); write_unlock (&parent->u.dir.lock);
down (&dir->i_sem); /* Grab it again because them's the rules */ down (&dir->i_sem); /* Grab it again because them's the rules */
devfs_put (de); devfs_put (de);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment