linux-kernel - Re: [REGRESSION][BISECTED] Crash with Bad page state for FUSE/Flatpak related applications since v6.13

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8a99f6bf3f0b5cb909f11539fb3b0ef0d65b3a73.camel@kernel.org>
Date: Mon, 10 Feb 2025 17:38:39 -0500
From: Jeff Layton <jlayton@...nel.org>
To: Matthew Wilcox <willy@...radead.org>, Josef Bacik <josef@...icpanda.com>
Cc: Joanne Koong <joannelkoong@...il.com>, Vlastimil Babka <vbabka@...e.cz>,
  Miklos Szeredi <miklos@...redi.hu>, Christian Heusel
 <christian@...sel.eu>, Miklos Szeredi <mszeredi@...hat.com>, 
	regressions@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, linux-mm <linux-mm@...ck.org>, Mantas
 Mikulėnas	 <grawity@...il.com>
Subject: Re: [REGRESSION][BISECTED] Crash with Bad page state for
 FUSE/Flatpak related applications since v6.13

On Mon, 2025-02-10 at 20:36 +0000, Matthew Wilcox wrote:
> On Mon, Feb 10, 2025 at 02:12:35PM -0500, Josef Bacik wrote:
> > From: Josef Bacik <josef@...icpanda.com>
> > Date: Mon, 10 Feb 2025 14:06:40 -0500
> > Subject: [PATCH] fuse: drop extra put of folio when using pipe splice
> > 
> > In 3eab9d7bc2f4 ("fuse: convert readahead to use folios"), I converted
> > us to using the new folio readahead code, which drops the reference on
> > the folio once it is locked, using an inferred reference on the folio.
> > Previously we held a reference on the folio for the entire duration of
> > the readpages call.
> > 
> > This is fine, however I failed to catch the case for splice pipe
> > responses where we will remove the old folio and splice in the new
> > folio.  Here we assumed that there is a reference held on the folio for
> > ap->folios, which is no longer the case.
> > 
> > To fix this, simply drop the extra put to keep us consistent with the
> > non-splice variation.  This will fix the UAF bug that was reported.
> > 
> > Link: https://lore.kernel.org/linux-fsdevel/2f681f48-00f5-4e09-8431-2b3dbfaa881e@heusel.eu/
> > Fixes: 3eab9d7bc2f4 ("fuse: convert readahead to use folios")
> > Signed-off-by: Josef Bacik <josef@...icpanda.com>
> > ---
> >  fs/fuse/dev.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 5b5f789b37eb..5bd6e2e184c0 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -918,8 +918,6 @@ static int fuse_try_move_page(struct fuse_copy_state *cs, struct page **pagep)
> >  	}
> >  
> >  	folio_unlock(oldfolio);
> > -	/* Drop ref for ap->pages[] array */
> > -	folio_put(oldfolio);
> >  	cs->len = 0;
> 
> But aren't we now leaking a reference to newfolio?  ie shouldn't
> we also:
> 
> -	folio_get(newfolio);
> 
> a few lines earlier?
> 


I think that ref was leaking without Josef's patch, but your proposed
fix seems correct to me. There is:

- 1 reference stolen from the pipe_buffer
- 1 reference taken for the pagecache in replace_page_cache_folio()
- the folio_get(newfolio) just after that

The pagecache ref doesn't count here, and we only need the reference
that was stolen from the pipe_buffer to replace the one in pagep.
-- 
Jeff Layton <jlayton@...nel.org>