[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

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

[Ginger]

Hi Markus,

Thanks for checking. Do the following changes match your expectation? Thanks.

Sincerely,
Ginger
---
 fs/ocfs2/localalloc.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 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;
  }

- osb->local_alloc_bh = alloc_bh;
- osb->local_alloc_state = OCFS2_LA_ENABLED;
+ scoped_guard(spinlock, &osb->osb_lock) {
+ osb->local_alloc_bh = alloc_bh;
+ osb->local_alloc_state = OCFS2_LA_ENABLED;
+ }

 bail:
  if (status < 0)
@@ -392,7 +394,9 @@ void ocfs2_shutdown_local_alloc(struct ocfs2_super *os
  goto out;
  }

- osb->local_alloc_state = OCFS2_LA_DISABLED;
+ scoped_guard(spinlock, &osb->osb_lock) {
+ osb->local_alloc_state = OCFS2_LA_DISABLED;
+ }

  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);
- osb->local_alloc_bh = NULL;
- osb->local_alloc_state = OCFS2_LA_UNUSED;
+ scoped_guard(spinlock, &osb->osb_lock) {
+ osb->local_alloc_bh = NULL;
+ osb->local_alloc_state = OCFS2_LA_UNUSED;
+ }

  status = ocfs2_sync_local_to_main(osb, handle, alloc_copy,
   main_bm_inode, main_bm_bh);
-- 
2.43.0

On Thu, Sep 24, 2026 at 6:55 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> …
> > Take osb->osb_lock when updating both fields.
>
> May lock guards be applied in affected function implementations?
>
> Regards,
> Markus

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

[Markus Elfring]

> Thanks for checking. Do the following changes match your expectation?

I guess that source code indentation should be adjusted according to the proposed
usage of scoped lock guards.
Will further patch versions follow for improved applications of scope-based resource management?

Regards,
Markus