Re: [PATCH] gfs2: Fix use-after-free in iomap inline data write path
Andreas Gruenbacher <[email protected]>
| Newsgroups | dev.linux.lists.gfs2,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAHc6FU7f3ffoma263VbQAowjQYoC1K6f_=Ubp-mP_R5pds2PeA@mail.gmail.com> |
On Fri, Jan 30, 2026 at 12:33 PM Andreas Gruenbacher <[email protected]> wrote: > Hello Deepanshu, > > On Fri, Jan 30, 2026 at 10:21 AM Deepanshu Kartikey > <[email protected]> wrote: > > The inline data buffer head (dibh) is being released prematurely in > > gfs2_iomap_begin() via release_metapath(), while iomap->inline_data > > still points to dibh->b_data. This causes a use-after-free when > > iomap_write_end_inline() later attempts to write to the inline data > > area. > > > > The bug sequence: > > 1. gfs2_iomap_begin() calls gfs2_meta_inode_buffer() to read inode > > metadata into dibh > > 2. Sets iomap->inline_data = dibh->b_data + sizeof(struct gfs2_dinode) > > 3. Calls release_metapath() which calls brelse(dibh), dropping refcount > > to 0 > > 4. kswapd reclaims the page (~39ms later in the syzbot report) > > 5. iomap_write_end_inline() tries to memcpy() to iomap->inline_data > > 6. KASAN detects use-after-free write to freed memory > > > > Fix by storing dibh in iomap->private and incrementing its refcount > > with get_bh() in gfs2_iomap_begin(). The buffer is then properly > > released in gfs2_iomap_end() after the inline write completes, > > ensuring the page stays alive for the entire iomap operation. > > I agree with this analysis and fix. Some nitpicking below. > > > Note: A C reproducer is not available for this issue. The fix is based > > on analysis of the KASAN report and code review showing the buffer head > > is freed before use. > > > > Reported-by: [email protected] > > Closes: https://syzkaller.appspot.com/bug?extid=ea1cd4aa4d1e98458a55 > > Signed-off-by: Deepanshu Kartikey <[email protected]> > > --- > > fs/gfs2/bmap.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c > > index 131091520de6..e70095809e4a 100644 > > --- a/fs/gfs2/bmap.c > > +++ b/fs/gfs2/bmap.c > > @@ -887,6 +887,8 @@ static int __gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length, > > sizeof(struct gfs2_dinode); > > iomap->type = IOMAP_INLINE; > > iomap->inline_data = dibh->b_data + sizeof(struct gfs2_dinode); > > + iomap->private = dibh; > > + get_bh(dibh); > > goto out; > > } > > > > @@ -1144,6 +1146,12 @@ static int gfs2_iomap_end(struct inode *inode, loff_t pos, loff_t length, > > struct gfs2_inode *ip = GFS2_I(inode); > > struct gfs2_sbd *sdp = GFS2_SB(inode); > > > > + /* Release buffer head for inline data */ > > + if (iomap->type == IOMAP_INLINE && iomap->private) { > > + brelse(iomap->private); > > + iomap->private = NULL; > > + } > > We're not using iomap->private for anything else and that field is > zeroed out automatically, so this can be simplified to: > > if (iomap->private) > brelse(iomap->private); And then we've also got iomap->private leaking a dibh reference in gfs2_iomap_get() and gfs2_iomap_alloc(). > > + > > switch (flags & (IOMAP_WRITE | IOMAP_ZERO)) { > > case IOMAP_WRITE: > > if (flags & IOMAP_DIRECT) > > -- > > 2.43.0 > > > > Thanks, > Andreas