[PATCH] ext3 [linux-2.6.2.]: accessing already freed inodes when under memory pressure

[Carsten Otte <cotte@freenet.de>]

Hi all,

recently we ran into a problem found during our 2.6. test activities on s390 
architecture. The problem occured running a glibc build on Linux 2.6.2. 
running in a z/VM virtual machine with 6 processors on a z990 Server 
(6Processor SMP, 64-bit, big endian).
We were able to identify ext3 as the cause of the problem with the following 
debugging patch:
diff -ruN linux-2.6.2/fs/ext3/super.c 
linux-2.6.2+bug_statement/fs/ext3/super.c
--- linux-2.6.2/fs/ext3/super.c 2004-02-19 12:52:01.000000000 +0100
+++ linux-2.6.2+bug_statement/fs/ext3/super.c   2004-02-19 12:51:35.000000000 
+0100
@@ -449,6 +449,7 @@
 
 static void ext3_destroy_inode(struct inode *inode)
 {
+       BUG_ON (!list_empty(&EXT3_I(inode)->i_orphan));
        kmem_cache_free(ext3_inode_cachep, EXT3_I(inode));
 }
 

The output of the BUG_ON statement shows, that prune_icache [fs/inode.c] picks 
an inode with i_count == 0 and calls (via dispose_list) clear_inode() and 
destroy_inode(). Because the inode is still in use by ext3 internally, ext3 
later on reads from or writes to freed memory causing random behavior of the 
system. I think, this BUG_ON should go into the vanilla kernel to prevent 
data corruption.

Above was so far only reproducable in rare conditions when the system was 
under heavy memory pressure. Therefore I continued to debug this further:
The problem seemed to be related to reference counting of inodes. Therefore I 
added a debugging patch to iput_final() [fs/inode.c], that checks if an inode 
that is being dropped is still in the ext3 orphan list. This situation is 
quite easy to reproduce, and has shown one example path were this happens:
A user process calls sys_unlink() [fs/namei.c], which increments i_count, 
calls vfs_unlink [fs/namei.c] and afterwards calls iput().
vfs_unlink [fs/namei.c] works with the dentry, calls i_op->unlink() (in this 
case ext3_unlink) and returns.
ext3_unlink [FILE] decrements i_nlink and adds the inode to the s_orphan list 
before returning.
After sys_unlink() has completed, the inode is still referenced by ext3 while 
i_count has the same value like before, which triggers the problem in case 
prune_icache would now choose this inode to be freed.

Possible ways to fix above problem is change reference counting for inodes or 
make the prune_icache function aware of the internal reference to the inode 
(preferably without knowing about the internal data structures of the 
filesystem which would be a layering violation). The patch below does take 
the 2nd approach:
- adds additional super_operation s_op->inode_busy() allowing VFS to query if 
an inode is still internally referenced by the fs.
- adds ext3_inode_busy() to ext3 that checks if he inode is still internally 
referenced
- changes prune_icache to query inode_busy() in case this s_op is implemented 
by the individual fs

Patch fixing the problem:
diff -ruN linux-2.6.2/fs/ext3/super.c linux-2.6.2+ext3fix/fs/ext3/super.c
--- linux-2.6.2/fs/ext3/super.c 2004-02-19 12:46:35.000000000 +0100
+++ linux-2.6.2+ext3fix/fs/ext3/super.c 2004-02-04 17:50:03.000000000 +0100
@@ -453,6 +453,14 @@
        kmem_cache_free(ext3_inode_cachep, EXT3_I(inode));
 }
 
+static int ext3_inode_busy(struct inode *inode)
+{
+       if (!list_empty(&EXT3_I(inode)->i_orphan)) 
+               return 1;
+       else
+               return 0;
+}
+
 static void init_once(void * foo, kmem_cache_t * cachep, unsigned long flags)
 {
        struct ext3_inode_info *ei = (struct ext3_inode_info *) foo;
@@ -510,6 +518,7 @@
 static struct super_operations ext3_sops = {
        .alloc_inode    = ext3_alloc_inode,
        .destroy_inode  = ext3_destroy_inode,
+       .inode_busy     = ext3_inode_busy,
        .read_inode     = ext3_read_inode,
        .write_inode    = ext3_write_inode,
        .dirty_inode    = ext3_dirty_inode,
diff -ruN linux-2.6.2/fs/inode.c linux-2.6.2+ext3fix/fs/inode.c
--- linux-2.6.2/fs/inode.c      2004-02-19 12:46:35.000000000 +0100
+++ linux-2.6.2+ext3fix/fs/inode.c      2004-01-28 09:02:25.000000000 +0100
@@ -391,6 +391,9 @@
                return 0;
        if (inode->i_data.nrpages)
                return 0;
+       if (inode->i_sb->s_op->inode_busy
+           && inode->i_sb->s_op->inode_busy(inode))
+               return 0;
        return 1;
 }
 
@@ -424,7 +427,9 @@
 
                inode = list_entry(inode_unused.prev, struct inode, i_list);
 
-               if (inode->i_state || atomic_read(&inode->i_count)) {
+               if (inode->i_state || atomic_read(&inode->i_count)
+                   || (inode->i_sb->s_op->inode_busy 
+                        && (inode->i_sb->s_op->inode_busy(inode)))) {
                        list_move(&inode->i_list, &inode_unused);
                        continue;
                }
diff -ruN linux-2.6.2/include/linux/fs.h 
linux-2.6.2+ext3fix/include/linux/fs.h
--- linux-2.6.2/include/linux/fs.h      2004-02-19 12:46:35.000000000 +0100
+++ linux-2.6.2+ext3fix/include/linux/fs.h      2004-02-18 18:45:15.000000000 
+0100
@@ -866,6 +866,7 @@
 struct super_operations {
        struct inode *(*alloc_inode)(struct super_block *sb);
        void (*destroy_inode)(struct inode *);
+       int  (*inode_busy)(struct inode *);
 
        void (*read_inode) (struct inode *);
   

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html