linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Viro <viro@math.psu.edu>
To: Linus Torvalds <torvalds@transmeta.com>
Cc: Richard Gooch <rgooch@ras.ucalgary.ca>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] one of $BIGNUM devfs races
Date: Mon, 6 Aug 2001 07:58:37 -0400 (EDT)	[thread overview]
Message-ID: <Pine.GSO.4.21.0108060723110.13716-100000@weyl.math.psu.edu> (raw)

	OK, folks - that's it.  By all reasonable standards a year _is_
sufficient time to fix an obvious race.  One in devfs/base.c::create_entry()
had been described to Richard more than a year ago.  While I respect the
"I'll do it myself, don't spoil the fun" stance, it's clearly over the
bleedin' top.  Patch for that one is in the end of posting.  Linus, see
if it looks sane for you.

	Richard, _please_, stop adding features and spend some of the
freed time fixing the long-standing security holes.  E.g., readlink()
on devfs is a big "boys, come and get some" sign on the kernel's arse.
_Anyone_ can crash the box with devfs mounted on it as soon as rmmod
removes a symlink.  Yes, that one had been pointed out to you only
a couple of months ago.  It's less than a year, but could we please
get it fixed in some reasonable time?  By the end of December or
something...

	When devfs went into the tree, the word was "at least it will
make people look at the code".  Well, it did.  Veni, vidi, vomere.
There are tons of bad races in devfs/base.c.  Reported to you many times -
just look through your mailbox.  Richard, please, either fix the crap
yourself or step down and admit that devfs is unmaintained.  Saying
that you'll fix it yourself is nice, but there's a point when it gets
really old.  And that point had been crossed _way_ back.

--- S8-pre4/fs/devfs/base.c	Sun Jul 29 01:54:47 2001
+++ /tmp/base.c	Mon Aug  6 07:14:09 2001
@@ -789,47 +789,70 @@
 static struct devfs_entry *create_entry (struct devfs_entry *parent,
 					 const char *name,unsigned int namelen)
 {
-    struct devfs_entry *new, **table;
+	struct devfs_entry *new;
 
-    /*  First ensure table size is enough  */
-    if (fs_info.num_inodes >= fs_info.table_size)
-    {
-	if ( ( table = kmalloc (sizeof *table *
-				(fs_info.table_size + INODE_TABLE_INC),
-				GFP_KERNEL) ) == NULL ) return NULL;
-	fs_info.table_size += INODE_TABLE_INC;
+	if (name && namelen<1)
+		namelen = strlen (name);
+
+	new = kmalloc(sizeof(*new) + namelen, GFP_KERNEL);
+
+	if (!new)
+		return NULL;
+
+	/*
+	 * Magic: this will set the ctime to zero, thus subsequent lookups
+	 * will trigger the call to <update_devfs_inode_from_entry>
+	 */
+	memset (new, 0, sizeof *new + namelen);
+	new->parent = parent;
+	if (name)
+		memcpy (new->name, name, namelen);
+	new->namelen = namelen;
+	new->inode.nlink = 1;
+
+	/*  Ensure table size is enough  */
+	while (fs_info.num_inodes >= fs_info.table_size) {
+		unsigned new_size = fs_info.table_size + INODE_TABLE_INC;
+		struct devfs_entry **table;
+
+		table = kmalloc(sizeof(*table) * new_size, GFP_KERNEL);
+
+		if (new_size <= fs_info.table_size) {
+			kfree(table);
+			continue;
+		}
+		if (!table) {
+			kfree(new);
+			return NULL;
+		}
+		fs_info.table_size = new_size;
+		if (!fs_info.table) {
+			fs_info.table = table;
+			break;
+		}
+		memcpy(table, fs_info.table, sizeof(*table)*fs_info.num_inodes);
+		kfree (fs_info.table);
+		fs_info.table = table;
 #ifdef CONFIG_DEVFS_DEBUG
-	if (devfs_debug & DEBUG_I_CREATE)
-	    printk ("%s: create_entry(): grew inode table to: %u entries\n",
-		    DEVFS_NAME, fs_info.table_size);
+		if (devfs_debug & DEBUG_I_CREATE)
+			printk("%s: create_entry(): grew inode table to:"
+				"%u entries\n", DEVFS_NAME, new_size);
 #endif
-	if (fs_info.table)
-	{
-	    memcpy (table, fs_info.table, sizeof *table *fs_info.num_inodes);
-	    kfree (fs_info.table);
 	}
-	fs_info.table = table;
-    }
-    if ( name && (namelen < 1) ) namelen = strlen (name);
-    if ( ( new = kmalloc (sizeof *new + namelen, GFP_KERNEL) ) == NULL )
-	return NULL;
-    /*  Magic: this will set the ctime to zero, thus subsequent lookups will
-	trigger the call to <update_devfs_inode_from_entry>  */
-    memset (new, 0, sizeof *new + namelen);
-    new->parent = parent;
-    if (name) memcpy (new->name, name, namelen);
-    new->namelen = namelen;
-    new->inode.ino = fs_info.num_inodes + FIRST_INODE;
-    new->inode.nlink = 1;
-    fs_info.table[fs_info.num_inodes] = new;
-    ++fs_info.num_inodes;
-    if (parent == NULL) return new;
-    new->prev = parent->u.dir.last;
-    /*  Insert into the parent directory's list of children  */
-    if (parent->u.dir.first == NULL) parent->u.dir.first = new;
-    else parent->u.dir.last->next = new;
-    parent->u.dir.last = new;
-    return new;
+
+	new->inode.ino = fs_info.num_inodes + FIRST_INODE;
+	fs_info.table[fs_info.num_inodes++] = new;
+
+	if (parent) {
+		new->prev = parent->u.dir.last;
+		/*  Insert into the parent directory's list of children  */
+		if (parent->u.dir.first)
+			parent->u.dir.last->next = new;
+		else
+			parent->u.dir.first = new;
+		parent->u.dir.last = new;
+	}
+	return new;
 }   /*  End Function create_entry  */
 
 static void update_devfs_inode_from_entry (struct devfs_entry *de)


             reply	other threads:[~2001-08-06 11:58 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-08-06 11:58 Alexander Viro [this message]
2001-08-06 16:47 ` [PATCH] one of $BIGNUM devfs races Richard Gooch
2001-08-06 16:56   ` Rik van Riel
2001-08-06 17:11   ` Richard Gooch
2001-08-06 23:50 ` Richard Gooch
2001-08-07  0:34   ` Alexander Viro
2001-08-07  0:51   ` Richard Gooch
2001-08-07  1:10     ` Alexander Viro
2001-08-07  1:27     ` Richard Gooch
2001-08-07  1:42       ` Alexander Viro
2001-08-07  2:00       ` Richard Gooch
2001-08-07  2:15         ` Alexander Viro
2001-08-07  5:17         ` Richard Gooch
2001-08-07  5:28           ` Alexander Viro
2001-08-07 10:21           ` Anton Altaparmakov
2001-08-07 17:09           ` Richard Gooch
2001-08-07 17:27             ` Arjan van de Ven
2001-08-07 17:28             ` Richard Gooch
2001-08-07  5:53       ` Richard Gooch
2001-08-07  6:23         ` Alexander Viro
2001-08-07  6:36         ` Richard Gooch
2001-08-07  6:40           ` Alexander Viro
2001-08-07  6:47           ` Richard Gooch
2001-08-07  7:31             ` Alexander Viro
2001-08-07 18:11             ` Richard Gooch
2001-08-07 18:23               ` Alexander Viro
2001-08-07 18:55               ` Richard Gooch
2001-08-07 19:10                 ` Alexander Viro
2001-08-07 22:13                 ` Richard Gooch
2001-08-06 23:59 Alan Cox
     [not found] ` <no.id>
2001-08-06 23:54   ` Alan Cox
2001-08-06 23:55   ` Richard Gooch
2001-08-06 23:59   ` Richard Gooch
2001-08-07 16:22   ` Alan Cox
2001-08-07 19:04   ` Alan Cox
2001-08-07 19:16     ` Alexander Viro
2001-08-08 21:16       ` H. Peter Anvin
2001-08-08 21:47         ` Alexander Viro
2001-08-08 23:29         ` Richard Gooch
2001-08-07 19:09   ` Richard Gooch
2001-08-07 19:20     ` 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=Pine.GSO.4.21.0108060723110.13716-100000@weyl.math.psu.edu \
    --to=viro@math.psu.edu \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rgooch@ras.ucalgary.ca \
    --cc=torvalds@transmeta.com \
    /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;
as well as URLs for NNTP newsgroup(s).