[Yaffs] [PATCH] Fix build error and race between inode put a…

Top Page
Attachments:
Message as email
+ (text/plain)
Delete this message
Reply to this message
Author: Jisheng Zhang
Date:  
To: Charles Manning, yaffs
Subject: [Yaffs] [PATCH] Fix build error and race between inode put and inode list iteration
First of all, fix build error, the inode should be iptr.

Secondly, properly fix the race between yaffs_flush_inodes and inode
put. Commit b4ce1bb1b46a ("Fix race yaffs_flush_inodes against iput")
tries to fix the race in yaffs_flush_inodes(), but it didn't fully
fix the race. Consider below case:

CPU0: yaffs_flush_inodes()            CPU1:
...
                        iput(iptr);
we hold the last reference of iptr now
iput(iptr); // now iptr is deleted.


spin_lock(&sb->s_inode_list_lock);
iptr = list_next_entry(iptr, member)) in list_for_each_entry() can't
get the next node, leads to an endless loop.

Fix this issue by keeping a reference to inode and putting the
reference only after we have safely resumed the scan of the inode list.

Signed-off-by: Jisheng Zhang <>
---
yaffs_vfs_single.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/yaffs_vfs_single.c b/yaffs_vfs_single.c
index 1abbfd8..384d809 100644
--- a/yaffs_vfs_single.c
+++ b/yaffs_vfs_single.c
@@ -1487,20 +1487,20 @@ static int yaffs_statfs(struct dentry *dentry, struct kstatfs *buf)

 static void yaffs_flush_inodes(struct super_block *sb)
 {
-    struct inode *iptr;
+    struct inode *iptr, *old_iptr = NULL;
     struct yaffs_obj *obj;
     struct yaffs_dev *dev = yaffs_super_to_dev(sb);


     spin_lock(&sb->s_inode_list_lock);
     list_for_each_entry(iptr, &sb->s_inodes, i_sb_list) {
-        spin_lock(&inode->i_lock);
+        spin_lock(&iptr->i_lock);
         if (iptr->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) {
-            spin_unlock(&inode->i_lock);
+            spin_unlock(&iptr->i_lock);
             continue;
         }


         __iget(iptr);
-        spin_unlock(&inode->i_lock);
+        spin_unlock(&iptr->i_lock);
         spin_unlock(&sb->s_inode_list_lock);


         obj = yaffs_inode_to_obj(iptr);
@@ -1510,13 +1510,25 @@ static void yaffs_flush_inodes(struct super_block *sb)
             yaffs_flush_file(obj, 1, 0, 0);
         }


+        /*
+         * We hold a reference to 'iptr' so it couldn't have been
+         * removed from s_inodes list while we dropped the
+         * s_inode_list_lock.  We cannot iput the iptr now as we can
+         * be holding the last reference and we cannot iput it under
+         * s_inode_list_lock. So we keep the reference and iput it
+         * later. And iput may call yaffs_delete_inode which will lock
+         * the yaffs gross lock so we should unlock it firstly and
+         * lock again after iput.
+         */
         yaffs_gross_unlock(dev);
-        iput(iptr);
+        iput(old_iptr);
+        old_iptr = iptr;
         yaffs_gross_lock(dev);


         spin_lock(&sb->s_inode_list_lock);
     }
     spin_unlock(&sb->s_inode_list_lock);
+    iput(old_iptr);
 }


static void yaffs_flush_super(struct super_block *sb, int do_checkpoint)
--
2.7.4