[PATCH] ocfs2: free replay slots in ocfs2_recovery_exit()

[PATCH] ocfs2: free replay slots in ocfs2_recovery_exit()

[Joseph Qi]

Commit ce2fcf1516d6 ("ocfs2: fix memory leak in ocfs2_mount_volume()")
added ocfs2_free_replay_slots() calls to the mount error paths
out_dismount and out_check_volume to fix a leak of osb->replay_map.
However these calls are unlocked while the bail path of the recovery
thread, which is woken up by out_dismount right before the call, also
frees the replay slots under osb->recovery_lock.  Both sides can thus
observe a non-NULL osb->replay_map and trigger a double free.

Fix this by moving ocfs2_free_replay_slots() into ocfs2_recovery_exit()
after ocfs2_recovery_disable(), which waits for a running recovery
thread to exit under osb->recovery_lock, and drop the unlocked call
sites.  Both ocfs2_dismount_volume() and the out_super path of
ocfs2_fill_super() call ocfs2_recovery_exit(), so the replay slots are
freed on every path.  Since super.c no longer references it, make
ocfs2_free_replay_slots() static again.

Fixes: ce2fcf1516d6 ("ocfs2: fix memory leak in ocfs2_mount_volume()")
Cc: <stable@vger.kernel.org>
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/journal.c | 3 ++-
 fs/ocfs2/journal.h | 1 -
 fs/ocfs2/super.c   | 5 +----
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index d8afbc1a76bb..a3938a03e93b 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -156,7 +156,7 @@ static void ocfs2_queue_replay_slots(struct ocfs2_super *osb,
 	replay_map->rm_state = REPLAY_DONE;
 }
 
-void ocfs2_free_replay_slots(struct ocfs2_super *osb)
+static void ocfs2_free_replay_slots(struct ocfs2_super *osb)
 {
 	struct ocfs2_replay_map *replay_map = osb->replay_map;
 
@@ -243,6 +243,7 @@ void ocfs2_recovery_exit(struct ocfs2_super *osb)
 	/* XXX: Should we bug if there are dirty entries? */
 
 	kfree(rm);
+	ocfs2_free_replay_slots(osb);
 }
 
 static int __ocfs2_recovery_map_test(struct ocfs2_super *osb,
diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
index f8b3b2a3d630..19fc920d26b1 100644
--- a/fs/ocfs2/journal.h
+++ b/fs/ocfs2/journal.h
@@ -151,7 +151,6 @@ void ocfs2_recovery_exit(struct ocfs2_super *osb);
 void ocfs2_recovery_disable_quota(struct ocfs2_super *osb);
 
 int ocfs2_compute_replay_slots(struct ocfs2_super *osb);
-void ocfs2_free_replay_slots(struct ocfs2_super *osb);
 /*
  *  Journal Control:
  *  Initialize, Load, Shutdown, Wipe a journal.
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index f785c39d1fb8..6a8092b65bb5 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -1162,7 +1162,6 @@ static int ocfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 out_dismount:
 	atomic_set(&osb->vol_state, VOLUME_DISABLED);
 	wake_up(&osb->osb_mount_event);
-	ocfs2_free_replay_slots(osb);
 	ocfs2_dismount_volume(sb, 1);
 	goto out;
 
@@ -1776,14 +1775,12 @@ static int ocfs2_mount_volume(struct super_block *sb)
 	status = ocfs2_truncate_log_init(osb);
 	if (status < 0) {
 		mlog_errno(status);
-		goto out_check_volume;
+		goto out_system_inodes;
 	}
 
 	ocfs2_super_unlock(osb, 1);
 	return 0;
 
-out_check_volume:
-	ocfs2_free_replay_slots(osb);
 out_system_inodes:
 	if (osb->local_alloc_state == OCFS2_LA_ENABLED)
 		ocfs2_shutdown_local_alloc(osb);
-- 
2.39.3

Re: [PATCH] ocfs2: free replay slots in ocfs2_recovery_exit()

[Joseph Qi]

On 8/28/26 6:01 PM, Joseph Qi wrote:
> Commit ce2fcf1516d6 ("ocfs2: fix memory leak in ocfs2_mount_volume()")
> added ocfs2_free_replay_slots() calls to the mount error paths
> out_dismount and out_check_volume to fix a leak of osb->replay_map.
> However these calls are unlocked while the bail path of the recovery
> thread, which is woken up by out_dismount right before the call, also
> frees the replay slots under osb->recovery_lock.  Both sides can thus
> observe a non-NULL osb->replay_map and trigger a double free.
> 
> Fix this by moving ocfs2_free_replay_slots() into ocfs2_recovery_exit()
> after ocfs2_recovery_disable(), which waits for a running recovery
> thread to exit under osb->recovery_lock, and drop the unlocked call
> sites.  Both ocfs2_dismount_volume() and the out_super path of
> ocfs2_fill_super() call ocfs2_recovery_exit(), so the replay slots are
> freed on every path.  Since super.c no longer references it, make
> ocfs2_free_replay_slots() static again.
> 

Sashiko has review comments:
https://sashiko.dev/#/patchset/20260828100114.530941-1-joseph.qi@linux.alibaba.com?part=1

This is because it is base on patch:
  "ocfs2: exit recovery thread on mount error path"

I'll resend them as a series.

Thanks,
Joseph

> Fixes: ce2fcf1516d6 ("ocfs2: fix memory leak in ocfs2_mount_volume()")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
>  fs/ocfs2/journal.c | 3 ++-
>  fs/ocfs2/journal.h | 1 -
>  fs/ocfs2/super.c   | 5 +----
>  3 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index d8afbc1a76bb..a3938a03e93b 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -156,7 +156,7 @@ static void ocfs2_queue_replay_slots(struct ocfs2_super *osb,
>  	replay_map->rm_state = REPLAY_DONE;
>  }
>  
> -void ocfs2_free_replay_slots(struct ocfs2_super *osb)
> +static void ocfs2_free_replay_slots(struct ocfs2_super *osb)
>  {
>  	struct ocfs2_replay_map *replay_map = osb->replay_map;
>  
> @@ -243,6 +243,7 @@ void ocfs2_recovery_exit(struct ocfs2_super *osb)
>  	/* XXX: Should we bug if there are dirty entries? */
>  
>  	kfree(rm);
> +	ocfs2_free_replay_slots(osb);
>  }
>  
>  static int __ocfs2_recovery_map_test(struct ocfs2_super *osb,
> diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
> index f8b3b2a3d630..19fc920d26b1 100644
> --- a/fs/ocfs2/journal.h
> +++ b/fs/ocfs2/journal.h
> @@ -151,7 +151,6 @@ void ocfs2_recovery_exit(struct ocfs2_super *osb);
>  void ocfs2_recovery_disable_quota(struct ocfs2_super *osb);
>  
>  int ocfs2_compute_replay_slots(struct ocfs2_super *osb);
> -void ocfs2_free_replay_slots(struct ocfs2_super *osb);
>  /*
>   *  Journal Control:
>   *  Initialize, Load, Shutdown, Wipe a journal.
> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
> index f785c39d1fb8..6a8092b65bb5 100644
> --- a/fs/ocfs2/super.c
> +++ b/fs/ocfs2/super.c
> @@ -1162,7 +1162,6 @@ static int ocfs2_fill_super(struct super_block *sb, struct fs_context *fc)
>  out_dismount:
>  	atomic_set(&osb->vol_state, VOLUME_DISABLED);
>  	wake_up(&osb->osb_mount_event);
> -	ocfs2_free_replay_slots(osb);
>  	ocfs2_dismount_volume(sb, 1);
>  	goto out;
>  
> @@ -1776,14 +1775,12 @@ static int ocfs2_mount_volume(struct super_block *sb)
>  	status = ocfs2_truncate_log_init(osb);
>  	if (status < 0) {
>  		mlog_errno(status);
> -		goto out_check_volume;
> +		goto out_system_inodes;
>  	}
>  
>  	ocfs2_super_unlock(osb, 1);
>  	return 0;
>  
> -out_check_volume:
> -	ocfs2_free_replay_slots(osb);
>  out_system_inodes:
>  	if (osb->local_alloc_state == OCFS2_LA_ENABLED)
>  		ocfs2_shutdown_local_alloc(osb);