Skip to contentSkip to conversation

fix NULL pointer dereference in xfs_log_force_umount

PATCH In review Christoph Hellwig 2 participants 2 messages Nov 21, 2008 XFS filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix NULL pointer dereference in xfs_log_force_umount

xfs_log_force_umount may be called very early during log recovery where

If we fail a buffer read in xlog_recover_do_inode_trans we abort the mount. But at that point log recovery has started delayed writeback of inode
buffers. As part of the aborted mount we try to flush out all delwri
buffers, but at that point we have already freed the superblock, and set mp->m_sb_bp to NULL, and xfs_log_force_umount which gets called after the inode buffer writeback trips over it.

Make xfs_log_force_umounr a little more careful when accessing mp->m_sb_bp to avoid this.

fs/xfs/xfs_log.c +5 -2
Index: xfs-2.6/fs/xfs/xfs_log.c
===================================================================
@@ -3525,7 +3525,8 @@ xfs_log_force_umount(
 	if (!log ||
 	    log->l_flags & XLOG_ACTIVE_RECOVERY) {
 		mp->m_flags |= XFS_MOUNT_FS_SHUTDOWN;
-		XFS_BUF_DONE(mp->m_sb_bp);
+		if (mp->m_sb_bp)
+			XFS_BUF_DONE(mp->m_sb_bp);
 		return 0;
 	}
 
@@ -3546,7 +3547,9 @@ xfs_log_force_umount(
 	spin_lock(&log->l_icloglock);
 	spin_lock(&log->l_grant_lock);
 	mp->m_flags |= XFS_MOUNT_FS_SHUTDOWN;
-	XFS_BUF_DONE(mp->m_sb_bp);
+	if (mp->m_sb_bp)
+		XFS_BUF_DONE(mp->m_sb_bp);
+
 	/*
 	 * This flag is sort of redundant because of the mount flag, but
 	 * it's good to maintain the separation between the log and the rest

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw
Quoted 11 lines · Christoph Hellwig

xfs_log_force_umount may be called very early during log recovery where

If we fail a buffer read in xlog_recover_do_inode_trans we abort the mount. But at that point log recovery has started delayed writeback of inode
buffers. As part of the aborted mount we try to flush out all delwri
buffers, but at that point we have already freed the superblock, and set mp->m_sb_bp to NULL, and xfs_log_force_umount which gets called after the inode buffer writeback trips over it.

Make xfs_log_force_umounr a little more careful when accessing mp->m_sb_bp to avoid this.

Seems fine (btw: s/unmounr/unmount/) ;)

-eric

Quoted 29 lines

Index: xfs-2.6/fs/xfs/xfs_log.c
===================================================================

fs/xfs/xfs_log.c +5 -2
@@ -3525,7 +3525,8 @@ xfs_log_force_umount(
 	if (!log ||
 	    log->l_flags & XLOG_ACTIVE_RECOVERY) {
 		mp->m_flags |= XFS_MOUNT_FS_SHUTDOWN;
-		XFS_BUF_DONE(mp->m_sb_bp);
+		if (mp->m_sb_bp)
+			XFS_BUF_DONE(mp->m_sb_bp);
 		return 0;
 	}
 
@@ -3546,7 +3547,9 @@ xfs_log_force_umount(
 	spin_lock(&log->l_icloglock);
 	spin_lock(&log->l_grant_lock);
 	mp->m_flags |= XFS_MOUNT_FS_SHUTDOWN;
-	XFS_BUF_DONE(mp->m_sb_bp);
+	if (mp->m_sb_bp)
+		XFS_BUF_DONE(mp->m_sb_bp);
+
 	/*
 	 * This flag is sort of redundant because of the mount flag, but
 	 * it's good to maintain the separation between the log and the rest
Quoted 5 lines

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs