[3/8] ocfs2: revert "ocfs2: fix NULL pointer dereference when dismount and ocfs2rec simultaneously"

Commit Message

Andrew Morton June 9, 2014, 8:04 p.m. UTC

From: Xue jiufei <[e-mail]>
Subject: ocfs2: revert "ocfs2: fix NULL pointer dereference when dismount and ocfs2rec simultaneously"

75f82eaa502c ("ocfs2: fix NULL pointer dereference when dismount and
ocfs2rec simultaneously") may cause umount hang while shutting down
truncate log.

The situation is as followes:
ocfs2_dismout_volume
-> ocfs2_recovery_exit
  -> free osb->recovery_map
-> ocfs2_truncate_shutdown
  -> lock global bitmap inode
    -> ocfs2_wait_for_recovery
	  -> check whether osb->recovery_map->rm_used is zero
	
Because osb->recovery_map is already freed, rm_used can be any other
values, so it may yield umount hang.

Signed-off-by: joyce.xue <[e-mail]>
Cc: Mark Fasheh <[e-mail]>
Cc: Joel Becker <[e-mail]>
Signed-off-by: Andrew Morton <[e-mail]>
---

 fs/ocfs2/super.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Mark Fasheh June 12, 2014, 11:17 p.m. UTC | #1

On Mon, Jun 09, 2014 at 01:04:02PM -0700, Andrew Morton wrote:
> From: Xue jiufei <[e-mail]>
> Subject: ocfs2: revert "ocfs2: fix NULL pointer dereference when dismount and ocfs2rec simultaneously"
> 
> 75f82eaa502c ("ocfs2: fix NULL pointer dereference when dismount and
> ocfs2rec simultaneously") may cause umount hang while shutting down
> truncate log.
> 
> The situation is as followes:
> ocfs2_dismout_volume
> -> ocfs2_recovery_exit
>   -> free osb->recovery_map
> -> ocfs2_truncate_shutdown
>   -> lock global bitmap inode
>     -> ocfs2_wait_for_recovery
> 	  -> check whether osb->recovery_map->rm_used is zero
> 	
> Because osb->recovery_map is already freed, rm_used can be any other
> values, so it may yield umount hang.

Good catch, thanks for this.

I'm thinking the original patch was just wrong and we need to make sure
everything related to file system maintenance is shut down completely
(including truncate log flushes) before turning off recovery.


Reviewed-by: Mark Fasheh <[e-mail]>
	--Mark

--
Mark Fasheh

Patch

diff -puN fs/ocfs2/super.c~ocfs2-revert-the-patch-fix-null-pointer-dereference-when-dismount-and-ocfs2rec-simultaneously fs/ocfs2/super.c
--- a/fs/ocfs2/super.c~ocfs2-revert-the-patch-fix-null-pointer-dereference-when-dismount-and-ocfs2rec-simultaneously
+++ a/fs/ocfs2/super.c
@@ -1925,15 +1925,11 @@  static void ocfs2_dismount_volume(struct
 
 	ocfs2_shutdown_local_alloc(osb);
 
+	ocfs2_truncate_log_shutdown(osb);
+
 	/* This will disable recovery and flush any recovery work. */
 	ocfs2_recovery_exit(osb);
 
-	/*
-	 * During dismount, when it recovers another node it will call
-	 * ocfs2_recover_orphans and queue delayed work osb_truncate_log_wq.
-	 */
-	ocfs2_truncate_log_shutdown(osb);
-
 	ocfs2_journal_shutdown(osb);
 
 	ocfs2_sync_blockdev(sb);