Re: [PATCH] ocfs2: Protect local_alloc_state updates in load/shutdown

[Heming Zhao <heming.zhao@suse.com>]

On Tue, Sep 22, 2026 at 02:00:48PM +0800, Ginger Li wrote:
> My static analyzer identified a potential issue in 'fs/ocfs2/localalloc.c':
> osb->local_alloc_state is documented as protected by osb->osb_lock in
> struct ocfs2_super, and most of the code follows that rule:
> ocfs2_local_alloc_seen_free_bits(), ocfs2_la_enable_worker() and
> ocfs2_recalc_la_window() all update the field with the lock held.
> 
> ocfs2_load_local_alloc() and ocfs2_shutdown_local_alloc() update
> local_alloc_state, and local_alloc_bh next to it, without taking osb_lock, so
> those stores can race with the reads and writes done by the local alloc
> reserve path.  Those two writers predate the locking convention and were
> never converted.
> 
> Take osb->osb_lock when updating both fields.
> 
> Fixes: ccd979bdbce9 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
> Signed-off-by: Ginger Li <ginger.jzllee@gmail.com>
> ---
>  fs/ocfs2/localalloc.c | 6 ++++++
>  1 file changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
> --- a/fs/ocfs2/localalloc.c
> +++ b/fs/ocfs2/localalloc.c
> @@ -342,8 +342,10 @@ int ocfs2_load_local_alloc(struct ocfs2_super *osb)
>  		goto bail;
>  	}
>  
> +	spin_lock(&osb->osb_lock);
>  	osb->local_alloc_bh = alloc_bh;
>  	osb->local_alloc_state = OCFS2_LA_ENABLED;
> +	spin_unlock(&osb->osb_lock);
>  

This function is triggered during the mount phase. The la (localalloc) is only
active after this point, and the fs is still in the initialization state, so no
inodes can be created. Therefore, we don't need to worry about any race conditions.

>  bail:
>  	if (status < 0)
> @@ -392,7 +394,9 @@ void ocfs2_shutdown_local_alloc(struct ocfs2_super *os
>  		goto out;
>  	}
>  
> +	spin_lock(&osb->osb_lock);
>  	osb->local_alloc_state = OCFS2_LA_DISABLED;
> +	spin_unlock(&osb->osb_lock);
>  
>  	ocfs2_resmap_uninit(&osb->osb_la_resmap);
>  
> @@ -441,8 +445,10 @@ void ocfs2_shutdown_local_alloc(struct ocfs2_super *os
>  	ocfs2_journal_dirty(handle, bh);
>  
>  	brelse(bh);
> +	spin_lock(&osb->osb_lock);
>  	osb->local_alloc_bh = NULL;
>  	osb->local_alloc_state = OCFS2_LA_UNUSED;
> +	spin_unlock(&osb->osb_lock);
>  
>  	status = ocfs2_sync_local_to_main(osb, handle, alloc_copy,
>  					  main_bm_inode, main_bm_bh);
> -- 
> 2.43.0
> 

When the code enters the umount phase, the VFS layer ensures that there are
no other references to the fs, so no race can occur at this time.

At last, if you have reproducible steps, please provide them to help us
understand the issue better.

Thanks,
Heming