[PATCH v2] gfs2: move brelse() after buffer head accesses

Hongling Zeng posted 1 patch 1 month ago
fs/gfs2/log.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH v2] gfs2: move brelse() after buffer head accesses
Posted by Hongling Zeng 1 month ago
The brelse() call happens before all buffer head accesses are complete.
While the reference counting prevents a real use-after-free in practice,
this pattern is error-prone for future maintenance.

Move the brelse() call to the end of the function to make the intent
clearer and eliminate potential static analysis warnings.

Suggested-by: Andreas Gruenbacher <agruenba@redhat.com>
Suggested-by: Andrew Price <anprice@redhat.com>
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 Change in v2:
 -Correct the commit.
---
 fs/gfs2/log.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 78bba8cc10b8..a92c84146de9 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -1038,7 +1038,6 @@ void gfs2_remove_from_journal(struct buffer_head *bh, int meta)
 			set_bit(TR_TOUCHED, &tr->tr_flags);
 		}
 		was_pinned = 1;
-		brelse(bh);
 	}
 	if (bd) {
 		if (bd->bd_tr) {
@@ -1056,6 +1055,8 @@ void gfs2_remove_from_journal(struct buffer_head *bh, int meta)
 	}
 	clear_buffer_dirty(bh);
 	clear_buffer_uptodate(bh);
+	if (was_pinned)
+		brelse(bh);
 }
 
 /**
-- 
2.25.1
Re: [PATCH v2] gfs2: move brelse() after buffer head accesses
Posted by Andreas Gruenbacher 1 month ago
On Tue, Aug 25, 2026 at 3:55 AM Hongling Zeng <zenghongling@kylinos.cn> wrote:
> The brelse() call happens before all buffer head accesses are complete.
> While the reference counting prevents a real use-after-free in practice,
> this pattern is error-prone for future maintenance.
>
> Move the brelse() call to the end of the function to make the intent
> clearer and eliminate potential static analysis warnings.
>
> Suggested-by: Andreas Gruenbacher <agruenba@redhat.com>
> Suggested-by: Andrew Price <anprice@redhat.com>
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>  Change in v2:
>  -Correct the commit.
> ---
>  fs/gfs2/log.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
> index 78bba8cc10b8..a92c84146de9 100644
> --- a/fs/gfs2/log.c
> +++ b/fs/gfs2/log.c
> @@ -1038,7 +1038,6 @@ void gfs2_remove_from_journal(struct buffer_head *bh, int meta)
>                         set_bit(TR_TOUCHED, &tr->tr_flags);
>                 }
>                 was_pinned = 1;
> -               brelse(bh);
>         }
>         if (bd) {
>                 if (bd->bd_tr) {
> @@ -1056,6 +1055,8 @@ void gfs2_remove_from_journal(struct buffer_head *bh, int meta)
>         }
>         clear_buffer_dirty(bh);
>         clear_buffer_uptodate(bh);
> +       if (was_pinned)
> +               brelse(bh);
>  }
>
>  /**
> --
> 2.25.1
>

Merged, thanks.

Andreas