Skip to contentSkip to conversation

ext3 use-after-free bugfix

PATCH In review Andrew Morton 2 participants 2 messages Dec 15, 2002 – Dec 16, 2002

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ext3 use-after-free bugfix

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(-)
fs/ext3/namei.c renamed from fs/ext3/namei.c~ext3-use-after-free +5 -6
@@ -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;
 }

_

#1 lore raw

Re: [patch] ext3 fixes

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

Quoted 2 lines

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?

Signature · 2 lines
Ciao,
Pascal