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

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

[Ginger Li]

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);
 
 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

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

[Markus Elfring]

…
> Take osb->osb_lock when updating both fields.

May lock guards be applied in affected function implementations?

Regards,
Markus