[patch] ext3 use-after-free bugfix

[patch] ext3 use-after-free bugfix

[Andrew Morton]

A change was made to ext3 in 2.4.20-pre9 which will cause the
filesystem to run ext3_mark_inode_dirty() against a freed inode.

This will occur when an application attempts to add a new file/directory
to the filesystem and encounters space or inode exhaustion.

The results of this are unpredictable.  Usually, nothing happens.  But
it can cause random memory corruption on SMP, and the kernel will crash
if compiled for "debug memory allocations".

The problem is that ext3_add_nondir() will do an iput() of the inode
on error (which frees the inode) but we then run ext3_mark_inode_dirty()
against it.

Fix it so that we only run ext3_mark_inode_dirty() if the inode was
successfully instantiated.



 fs/ext3/namei.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

--- 24/fs/ext3/namei.c~ext3-use-after-free	Sun Dec 15 11:27:50 2002
+++ 24-akpm/fs/ext3/namei.c	Sun Dec 15 11:27:50 2002
@@ -429,8 +429,11 @@ static int ext3_add_nondir(handle_t *han
 {
 	int err = ext3_add_entry(handle, dentry, inode);
 	if (!err) {
-		d_instantiate(dentry, inode);
-		return 0;
+		err = ext3_mark_inode_dirty(handle, inode);
+		if (err == 0) {
+			d_instantiate(dentry, inode);
+			return 0;
+		}
 	}
 	ext3_dec_count(handle, inode);
 	iput(inode);
@@ -465,7 +468,6 @@ static int ext3_create (struct inode * d
 		inode->i_fop = &ext3_file_operations;
 		inode->i_mapping->a_ops = &ext3_aops;
 		err = ext3_add_nondir(handle, dentry, inode);
-		ext3_mark_inode_dirty(handle, inode);
 	}
 	ext3_journal_stop(handle, dir);
 	return err;
@@ -490,7 +492,6 @@ static int ext3_mknod (struct inode * di
 	if (!IS_ERR(inode)) {
 		init_special_inode(inode, mode, rdev);
 		err = ext3_add_nondir(handle, dentry, inode);
-		ext3_mark_inode_dirty(handle, inode);
 	}
 	ext3_journal_stop(handle, dir);
 	return err;
@@ -934,7 +935,6 @@ static int ext3_symlink (struct inode * 
 	}
 	inode->u.ext3_i.i_disksize = inode->i_size;
 	err = ext3_add_nondir(handle, dentry, inode);
-	ext3_mark_inode_dirty(handle, inode);
 out_stop:
 	ext3_journal_stop(handle, dir);
 	return err;
@@ -971,7 +971,6 @@ static int ext3_link (struct dentry * ol
 	atomic_inc(&inode->i_count);
 
 	err = ext3_add_nondir(handle, dentry, inode);
-	ext3_mark_inode_dirty(handle, inode);
 	ext3_journal_stop(handle, dir);
 	return err;
 }

_

Re: [patch] ext3 fixes

[Pascal Schmidt]

On Sun, 15 Dec 2002 21:30:16 +0100, you wrote in linux.kernel:

> Fix it so that we only run ext3_mark_inode_dirty() if the inode was
> successfully instantiated.

After applying your three ext3 fixes to 2.4.20 and rebooting with
data=journal, I get the following message in dmesg which does not
appear with clean 2.4.20 (no matter whether data=ordered or
data=journal):

blk: queue c0370520, I/O limit 4095Mb (mask 0xffffffff)

It appears just after the messages for mounting my ext3 filesystems:

kjournald starting.  Commit interval 5 seconds
EXT3-fs: mounted filesystem with journal data mode.
VFS: Mounted root (ext3 filesystem) readonly.
Freeing unused kernel memory: 256k freed
EXT3 FS 2.4-0.9.19, 19 August 2002 on ide0(3,65), internal journal
kjournald starting.  Commit interval 5 seconds
EXT3 FS 2.4-0.9.19, 19 August 2002 on ide0(3,71), internal journal
EXT3-fs: mounted filesystem with journal data mode.
kjournald starting.  Commit interval 5 seconds
EXT3 FS 2.4-0.9.19, 19 August 2002 on ide0(3,69), internal journal
EXT3-fs: mounted filesystem with journal data mode.
kjournald starting.  Commit interval 5 seconds
EXT3 FS 2.4-0.9.19, 19 August 2002 on ide0(3,70), internal journal
EXT3-fs: mounted filesystem with journal data mode.

Is the message just for information or should I worry?

-- 
Ciao,
Pascal