jfs: hold LOG_LOCK on umount to avoid null-ptr-deref

write_special_inodes() function iterate through the log->sb_list and
access the sbi fields, which can be set to NULL concurrently by umount.

Fix concurrency issue by holding LOG_LOCK and checking for NULL.

Reported-by: syzbot+e14b1036481911ae4d77@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e14b1036481911ae4d77
Signed-off-by: Helen Koike <koike@igalia.com>
Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>

diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
index 472b6500e8175151eea4c14835ac54f24587fd07..306165e61438cbfd9d1cb5a763e178d60b7883dc 100644 (file)
--- a/fs/jfs/jfs_logmgr.c
+++ b/fs/jfs/jfs_logmgr.c
@@ -74,12 +74,6 @@ static struct lbuf *log_redrive_list;
 static DEFINE_SPINLOCK(log_redrive_lock);
 
 
-/*
- *     log read/write serialization (per log)
- */
-#define LOG_LOCK_INIT(log)     mutex_init(&(log)->loglock)
-#define LOG_LOCK(log)          mutex_lock(&((log)->loglock))
-#define LOG_UNLOCK(log)                mutex_unlock(&((log)->loglock))
 
 
 /*
@@ -204,9 +198,13 @@ static void write_special_inodes(struct jfs_log *log,
        struct jfs_sb_info *sbi;
 
        list_for_each_entry(sbi, &log->sb_list, log_list) {
-               writer(sbi->ipbmap->i_mapping);
-               writer(sbi->ipimap->i_mapping);
-               writer(sbi->direct_inode->i_mapping);
+               /* These pointers can be NULL before list_del during umount */
+               if (sbi->ipbmap)
+                       writer(sbi->ipbmap->i_mapping);
+               if (sbi->ipimap)
+                       writer(sbi->ipimap->i_mapping);
+               if (sbi->direct_inode)
+                       writer(sbi->direct_inode->i_mapping);
        }
 }
 

diff --git a/fs/jfs/jfs_logmgr.h b/fs/jfs/jfs_logmgr.h
index 8b8994e48cd080b4df8084108e89c06c5ce36417..09e0ef6aeccefd41eecca2e8697d62df80064f03 100644 (file)
--- a/fs/jfs/jfs_logmgr.h
+++ b/fs/jfs/jfs_logmgr.h
@@ -402,6 +402,13 @@ struct jfs_log {
        int no_integrity;       /* 3: flag to disable journaling to disk */
 };
 
+/*
+ * log read/write serialization (per log)
+ */
+#define LOG_LOCK_INIT(log)     mutex_init(&(log)->loglock)
+#define LOG_LOCK(log)          mutex_lock(&((log)->loglock))
+#define LOG_UNLOCK(log)                mutex_unlock(&((log)->loglock))
+
 /*
  * Log flag
  */

diff --git a/fs/jfs/jfs_umount.c b/fs/jfs/jfs_umount.c
index 8ec43f53f68655ed2663986800c22fb7e7173064..18569f1eaabdbb73370a3711f28dc73a627e66da 100644 (file)
--- a/fs/jfs/jfs_umount.c
+++ b/fs/jfs/jfs_umount.c
@@ -20,6 +20,7 @@
 #include "jfs_superblock.h"
 #include "jfs_dmap.h"
 #include "jfs_imap.h"
+#include "jfs_logmgr.h"
 #include "jfs_metapage.h"
 #include "jfs_debug.h"
 
@@ -57,6 +58,12 @@ int jfs_umount(struct super_block *sb)
                 */
                jfs_flush_journal(log, 2);
 
+       /*
+        * Hold log lock so write_special_inodes (lmLogSync) cannot see
+        * this sbi with a NULL inode pointer while iterating log->sb_list.
+        */
+       if (log)
+               LOG_LOCK(log);
        /*
         * close fileset inode allocation map (aka fileset inode)
         */
@@ -95,6 +102,9 @@ int jfs_umount(struct super_block *sb)
         */
        filemap_write_and_wait(sbi->direct_inode->i_mapping);
 
+       if (log)
+               LOG_UNLOCK(log);
+
        /*
         * ensure all file system file pages are propagated to their
         * home blocks on disk (and their in-memory buffer pages are