Merge git://git.kernel.org/pub/scm/linux/kernel/git/hch/xfs-icache-races
authorLinus Torvalds <torvalds@linux-foundation.org>
Sat, 8 Aug 2009 01:53:44 +0000 (18:53 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Sat, 8 Aug 2009 01:53:44 +0000 (18:53 -0700)
* git://git.kernel.org/pub/scm/linux/kernel/git/hch/xfs-icache-races:
  xfs: fix freeing of inodes not yet added to the inode cache
  vfs: add __destroy_inode
  vfs: fix inode_init_always calling convention

fs/inode.c
fs/xfs/xfs_iget.c
fs/xfs/xfs_inode.h
include/linux/fs.h

index 901bad1e5f1210cce8f2bc2a474345f5b1a4072a..ae7b67e48661b0f078679a67d6f0c1d40847bf06 100644 (file)
@@ -120,12 +120,11 @@ static void wake_up_inode(struct inode *inode)
  * These are initializations that need to be done on every inode
  * allocation as the fields are not initialised by slab allocation.
  */
-struct inode *inode_init_always(struct super_block *sb, struct inode *inode)
+int inode_init_always(struct super_block *sb, struct inode *inode)
 {
        static const struct address_space_operations empty_aops;
        static struct inode_operations empty_iops;
        static const struct file_operations empty_fops;
-
        struct address_space *const mapping = &inode->i_data;
 
        inode->i_sb = sb;
@@ -152,7 +151,7 @@ struct inode *inode_init_always(struct super_block *sb, struct inode *inode)
        inode->dirtied_when = 0;
 
        if (security_inode_alloc(inode))
-               goto out_free_inode;
+               goto out;
 
        /* allocate and initialize an i_integrity */
        if (ima_inode_alloc(inode))
@@ -198,16 +197,12 @@ struct inode *inode_init_always(struct super_block *sb, struct inode *inode)
        inode->i_fsnotify_mask = 0;
 #endif
 
-       return inode;
+       return 0;
 
 out_free_security:
        security_inode_free(inode);
-out_free_inode:
-       if (inode->i_sb->s_op->destroy_inode)
-               inode->i_sb->s_op->destroy_inode(inode);
-       else
-               kmem_cache_free(inode_cachep, (inode));
-       return NULL;
+out:
+       return -ENOMEM;
 }
 EXPORT_SYMBOL(inode_init_always);
 
@@ -220,12 +215,21 @@ static struct inode *alloc_inode(struct super_block *sb)
        else
                inode = kmem_cache_alloc(inode_cachep, GFP_KERNEL);
 
-       if (inode)
-               return inode_init_always(sb, inode);
-       return NULL;
+       if (!inode)
+               return NULL;
+
+       if (unlikely(inode_init_always(sb, inode))) {
+               if (inode->i_sb->s_op->destroy_inode)
+                       inode->i_sb->s_op->destroy_inode(inode);
+               else
+                       kmem_cache_free(inode_cachep, inode);
+               return NULL;
+       }
+
+       return inode;
 }
 
-void destroy_inode(struct inode *inode)
+void __destroy_inode(struct inode *inode)
 {
        BUG_ON(inode_has_buffers(inode));
        ima_inode_free(inode);
@@ -237,13 +241,17 @@ void destroy_inode(struct inode *inode)
        if (inode->i_default_acl && inode->i_default_acl != ACL_NOT_CACHED)
                posix_acl_release(inode->i_default_acl);
 #endif
+}
+EXPORT_SYMBOL(__destroy_inode);
+
+void destroy_inode(struct inode *inode)
+{
+       __destroy_inode(inode);
        if (inode->i_sb->s_op->destroy_inode)
                inode->i_sb->s_op->destroy_inode(inode);
        else
                kmem_cache_free(inode_cachep, (inode));
 }
-EXPORT_SYMBOL(destroy_inode);
-
 
 /*
  * These are initializations that only need to be done
index 5fcec6f020a7ddff25df7e54e58750ca651733b1..34ec86923f7e354044fbe9ffa27a77126ce354cf 100644 (file)
@@ -64,6 +64,10 @@ xfs_inode_alloc(
        ip = kmem_zone_alloc(xfs_inode_zone, KM_SLEEP);
        if (!ip)
                return NULL;
+       if (inode_init_always(mp->m_super, VFS_I(ip))) {
+               kmem_zone_free(xfs_inode_zone, ip);
+               return NULL;
+       }
 
        ASSERT(atomic_read(&ip->i_iocount) == 0);
        ASSERT(atomic_read(&ip->i_pincount) == 0);
@@ -105,17 +109,6 @@ xfs_inode_alloc(
 #ifdef XFS_DIR2_TRACE
        ip->i_dir_trace = ktrace_alloc(XFS_DIR2_KTRACE_SIZE, KM_NOFS);
 #endif
-       /*
-       * Now initialise the VFS inode. We do this after the xfs_inode
-       * initialisation as internal failures will result in ->destroy_inode
-       * being called and that will pass down through the reclaim path and
-       * free the XFS inode. This path requires the XFS inode to already be
-       * initialised. Hence if this call fails, the xfs_inode has already
-       * been freed and we should not reference it at all in the error
-       * handling.
-       */
-       if (!inode_init_always(mp->m_super, VFS_I(ip)))
-               return NULL;
 
        /* prevent anyone from using this yet */
        VFS_I(ip)->i_state = I_NEW|I_LOCK;
@@ -123,6 +116,71 @@ xfs_inode_alloc(
        return ip;
 }
 
+STATIC void
+xfs_inode_free(
+       struct xfs_inode        *ip)
+{
+       switch (ip->i_d.di_mode & S_IFMT) {
+       case S_IFREG:
+       case S_IFDIR:
+       case S_IFLNK:
+               xfs_idestroy_fork(ip, XFS_DATA_FORK);
+               break;
+       }
+
+       if (ip->i_afp)
+               xfs_idestroy_fork(ip, XFS_ATTR_FORK);
+
+#ifdef XFS_INODE_TRACE
+       ktrace_free(ip->i_trace);
+#endif
+#ifdef XFS_BMAP_TRACE
+       ktrace_free(ip->i_xtrace);
+#endif
+#ifdef XFS_BTREE_TRACE
+       ktrace_free(ip->i_btrace);
+#endif
+#ifdef XFS_RW_TRACE
+       ktrace_free(ip->i_rwtrace);
+#endif
+#ifdef XFS_ILOCK_TRACE
+       ktrace_free(ip->i_lock_trace);
+#endif
+#ifdef XFS_DIR2_TRACE
+       ktrace_free(ip->i_dir_trace);
+#endif
+
+       if (ip->i_itemp) {
+               /*
+                * Only if we are shutting down the fs will we see an
+                * inode still in the AIL. If it is there, we should remove
+                * it to prevent a use-after-free from occurring.
+                */
+               xfs_log_item_t  *lip = &ip->i_itemp->ili_item;
+               struct xfs_ail  *ailp = lip->li_ailp;
+
+               ASSERT(((lip->li_flags & XFS_LI_IN_AIL) == 0) ||
+                                      XFS_FORCED_SHUTDOWN(ip->i_mount));
+               if (lip->li_flags & XFS_LI_IN_AIL) {
+                       spin_lock(&ailp->xa_lock);
+                       if (lip->li_flags & XFS_LI_IN_AIL)
+                               xfs_trans_ail_delete(ailp, lip);
+                       else
+                               spin_unlock(&ailp->xa_lock);
+               }
+               xfs_inode_item_destroy(ip);
+               ip->i_itemp = NULL;
+       }
+
+       /* asserts to verify all state is correct here */
+       ASSERT(atomic_read(&ip->i_iocount) == 0);
+       ASSERT(atomic_read(&ip->i_pincount) == 0);
+       ASSERT(!spin_is_locked(&ip->i_flags_lock));
+       ASSERT(completion_done(&ip->i_flush));
+
+       kmem_zone_free(xfs_inode_zone, ip);
+}
+
 /*
  * Check the validity of the inode we just found it the cache
  */
@@ -167,7 +225,7 @@ xfs_iget_cache_hit(
                 * errors cleanly, then tag it so it can be set up correctly
                 * later.
                 */
-               if (!inode_init_always(mp->m_super, VFS_I(ip))) {
+               if (inode_init_always(mp->m_super, VFS_I(ip))) {
                        error = ENOMEM;
                        goto out_error;
                }
@@ -299,7 +357,8 @@ out_preload_end:
        if (lock_flags)
                xfs_iunlock(ip, lock_flags);
 out_destroy:
-       xfs_destroy_inode(ip);
+       __destroy_inode(VFS_I(ip));
+       xfs_inode_free(ip);
        return error;
 }
 
@@ -504,62 +563,7 @@ xfs_ireclaim(
        xfs_qm_dqdetach(ip);
        xfs_iunlock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
 
-       switch (ip->i_d.di_mode & S_IFMT) {
-       case S_IFREG:
-       case S_IFDIR:
-       case S_IFLNK:
-               xfs_idestroy_fork(ip, XFS_DATA_FORK);
-               break;
-       }
-
-       if (ip->i_afp)
-               xfs_idestroy_fork(ip, XFS_ATTR_FORK);
-
-#ifdef XFS_INODE_TRACE
-       ktrace_free(ip->i_trace);
-#endif
-#ifdef XFS_BMAP_TRACE
-       ktrace_free(ip->i_xtrace);
-#endif
-#ifdef XFS_BTREE_TRACE
-       ktrace_free(ip->i_btrace);
-#endif
-#ifdef XFS_RW_TRACE
-       ktrace_free(ip->i_rwtrace);
-#endif
-#ifdef XFS_ILOCK_TRACE
-       ktrace_free(ip->i_lock_trace);
-#endif
-#ifdef XFS_DIR2_TRACE
-       ktrace_free(ip->i_dir_trace);
-#endif
-       if (ip->i_itemp) {
-               /*
-                * Only if we are shutting down the fs will we see an
-                * inode still in the AIL. If it is there, we should remove
-                * it to prevent a use-after-free from occurring.
-                */
-               xfs_log_item_t  *lip = &ip->i_itemp->ili_item;
-               struct xfs_ail  *ailp = lip->li_ailp;
-
-               ASSERT(((lip->li_flags & XFS_LI_IN_AIL) == 0) ||
-                                      XFS_FORCED_SHUTDOWN(ip->i_mount));
-               if (lip->li_flags & XFS_LI_IN_AIL) {
-                       spin_lock(&ailp->xa_lock);
-                       if (lip->li_flags & XFS_LI_IN_AIL)
-                               xfs_trans_ail_delete(ailp, lip);
-                       else
-                               spin_unlock(&ailp->xa_lock);
-               }
-               xfs_inode_item_destroy(ip);
-               ip->i_itemp = NULL;
-       }
-       /* asserts to verify all state is correct here */
-       ASSERT(atomic_read(&ip->i_iocount) == 0);
-       ASSERT(atomic_read(&ip->i_pincount) == 0);
-       ASSERT(!spin_is_locked(&ip->i_flags_lock));
-       ASSERT(completion_done(&ip->i_flush));
-       kmem_zone_free(xfs_inode_zone, ip);
+       xfs_inode_free(ip);
 }
 
 /*
index 1804f866a71d285b760db6bd5df1574676ecb898..65f24a3cc9922a174b8989c276691e01b1e56621 100644 (file)
@@ -309,23 +309,6 @@ static inline struct inode *VFS_I(struct xfs_inode *ip)
        return &ip->i_vnode;
 }
 
-/*
- * Get rid of a partially initialized inode.
- *
- * We have to go through destroy_inode to make sure allocations
- * from init_inode_always like the security data are undone.
- *
- * We mark the inode bad so that it takes the short cut in
- * the reclaim path instead of going through the flush path
- * which doesn't make sense for an inode that has never seen the
- * light of day.
- */
-static inline void xfs_destroy_inode(struct xfs_inode *ip)
-{
-       make_bad_inode(VFS_I(ip));
-       return destroy_inode(VFS_I(ip));
-}
-
 /*
  * i_flags helper functions
  */
index a36ffa5a77a4b2cbc45628e5eda30d4071828f6a..67888a9e06558d45d0ff0fc2fd97072af9aed5c8 100644 (file)
@@ -2137,7 +2137,7 @@ extern loff_t default_llseek(struct file *file, loff_t offset, int origin);
 
 extern loff_t vfs_llseek(struct file *file, loff_t offset, int origin);
 
-extern struct inode * inode_init_always(struct super_block *, struct inode *);
+extern int inode_init_always(struct super_block *, struct inode *);
 extern void inode_init_once(struct inode *);
 extern void inode_add_to_lists(struct super_block *, struct inode *);
 extern void iput(struct inode *);
@@ -2164,6 +2164,7 @@ extern void __iget(struct inode * inode);
 extern void iget_failed(struct inode *);
 extern void clear_inode(struct inode *);
 extern void destroy_inode(struct inode *);
+extern void __destroy_inode(struct inode *);
 extern struct inode *new_inode(struct super_block *);
 extern int should_remove_suid(struct dentry *);
 extern int file_remove_suid(struct file *);