All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Price <anprice@redhat.com>
To: Andreas Gruenbacher <agruenba@redhat.com>
Cc: Hongling Zeng <zenghongling@kylinos.cn>,
	swhiteho@redhat.com, gregkh@linuxfoundation.org,
	gfs2@lists.linux.dev, linux-kernel@vger.kernel.org,
	zhongling0719@126.com
Subject: Re: [PATCH] gfs2: Fix use-after-free in gfs2_remove_from_journal()
Date: Mon, 24 Aug 2026 15:44:53 +0100	[thread overview]
Message-ID: <9801cd18-e25c-406b-af05-ab0b21184b67@redhat.com> (raw)
In-Reply-To: <CAHc6FU6b00qRUXzTR27TL0u4Nf39zzvrCUjjFAa7ttwWYtRBmQ@mail.gmail.com>

On 24/08/2026 15:20, Andreas Gruenbacher wrote:
> Andy,
> 
> On Wed, Jun 17, 2026 at 12:52 PM Andrew Price <anprice@redhat.com> wrote:
>> On 17/06/2026 10:01, Hongling Zeng wrote:
>>> The function calls brelse(bh) but then continues to access
>>> the buffer head through bh->b_private, clear_buffer_dirty(),
>>> and clear_buffer_uptodate().
>>>
>>> After brelse() decreases the reference count, the buffer head
>>> may be freed, making the subsequent accesses use-after-free.
>>
>> When buffers are pinned their refcount is incremented and the brelse() here is only called for pinned buffers so I'm not convinced that there's a bug.
> 
> Not sure what you mean by that. The patch still only calls brelse() on
> buffers it unpins, it only changes when it does it.

The context is not fresh in my mind but I suppose was trying to locate the UAF that the patch reportedly fixed and I couldn't find one.

>> Callers of gfs2_remove_from_journal() also use the bh afterwards so if there was a use-after-free this patch wouldn't fix it.
> 
> Do we have any callers that do that when they don't clearly still hold
> a reference? I don't think so.
> 
> In any case, if there are any callers that don't certainly hold an
> extra reference, we need to be careful in gfs2_remove_from_journal().
> This patch achieves that, so I'm inclined to merge it.
No objections here. I'd like the commit description to be clear about whether it's fixing a bug or just cleaning up a static analysis warning.

Andy


  reply	other threads:[~2026-08-24 14:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-17  9:01 [PATCH] gfs2: Fix use-after-free in gfs2_remove_from_journal() Hongling Zeng
2026-06-17 10:51 ` Andrew Price
2026-06-18  1:50   ` Hongling Zeng
2026-08-24 15:35     ` Andreas Gruenbacher
2026-08-25  1:35       ` Hongling Zeng
2026-08-25 12:27         ` Andreas Gruenbacher
2026-08-24 14:20   ` Andreas Gruenbacher
2026-08-24 14:44     ` Andrew Price [this message]
2026-08-24 15:45 ` Andreas Gruenbacher
  -- strict thread matches above, loose matches on Subject: below --
2026-06-01  2:18 Hongling Zeng
2026-06-01  5:46 ` Greg KH
2026-05-22  9:43 Hongling Zeng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9801cd18-e25c-406b-af05-ab0b21184b67@redhat.com \
    --to=anprice@redhat.com \
    --cc=agruenba@redhat.com \
    --cc=gfs2@lists.linux.dev \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=swhiteho@redhat.com \
    --cc=zenghongling@kylinos.cn \
    --cc=zhongling0719@126.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.