Skip to content

Commit ca5848a

Browse files
helen-fornazierkleikamp
authored andcommitted
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>
1 parent b15e431 commit ca5848a

3 files changed

Lines changed: 24 additions & 9 deletions

File tree

fs/jfs/jfs_logmgr.c

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,6 @@ static struct lbuf *log_redrive_list;
7474
static DEFINE_SPINLOCK(log_redrive_lock);
7575

7676

77-
/*
78-
* log read/write serialization (per log)
79-
*/
80-
#define LOG_LOCK_INIT(log) mutex_init(&(log)->loglock)
81-
#define LOG_LOCK(log) mutex_lock(&((log)->loglock))
82-
#define LOG_UNLOCK(log) mutex_unlock(&((log)->loglock))
8377

8478

8579
/*
@@ -204,9 +198,13 @@ static void write_special_inodes(struct jfs_log *log,
204198
struct jfs_sb_info *sbi;
205199

206200
list_for_each_entry(sbi, &log->sb_list, log_list) {
207-
writer(sbi->ipbmap->i_mapping);
208-
writer(sbi->ipimap->i_mapping);
209-
writer(sbi->direct_inode->i_mapping);
201+
/* These pointers can be NULL before list_del during umount */
202+
if (sbi->ipbmap)
203+
writer(sbi->ipbmap->i_mapping);
204+
if (sbi->ipimap)
205+
writer(sbi->ipimap->i_mapping);
206+
if (sbi->direct_inode)
207+
writer(sbi->direct_inode->i_mapping);
210208
}
211209
}
212210

fs/jfs/jfs_logmgr.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,13 @@ struct jfs_log {
402402
int no_integrity; /* 3: flag to disable journaling to disk */
403403
};
404404

405+
/*
406+
* log read/write serialization (per log)
407+
*/
408+
#define LOG_LOCK_INIT(log) mutex_init(&(log)->loglock)
409+
#define LOG_LOCK(log) mutex_lock(&((log)->loglock))
410+
#define LOG_UNLOCK(log) mutex_unlock(&((log)->loglock))
411+
405412
/*
406413
* Log flag
407414
*/

fs/jfs/jfs_umount.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "jfs_superblock.h"
2121
#include "jfs_dmap.h"
2222
#include "jfs_imap.h"
23+
#include "jfs_logmgr.h"
2324
#include "jfs_metapage.h"
2425
#include "jfs_debug.h"
2526

@@ -57,6 +58,12 @@ int jfs_umount(struct super_block *sb)
5758
*/
5859
jfs_flush_journal(log, 2);
5960

61+
/*
62+
* Hold log lock so write_special_inodes (lmLogSync) cannot see
63+
* this sbi with a NULL inode pointer while iterating log->sb_list.
64+
*/
65+
if (log)
66+
LOG_LOCK(log);
6067
/*
6168
* close fileset inode allocation map (aka fileset inode)
6269
*/
@@ -95,6 +102,9 @@ int jfs_umount(struct super_block *sb)
95102
*/
96103
filemap_write_and_wait(sbi->direct_inode->i_mapping);
97104

105+
if (log)
106+
LOG_UNLOCK(log);
107+
98108
/*
99109
* ensure all file system file pages are propagated to their
100110
* home blocks on disk (and their in-memory buffer pages are

0 commit comments

Comments
 (0)