ACK: [SRU][J/I/H/F][PATCH 0/1] Drop "UBUNTU: SAUCE: cachefiles: Page leaking in cachefiles_read_backing_file while vmscan is active"

[Thadeu Lima de Souza Cascardo]

On Tue, Oct 19, 2021 at 05:05:39PM +0200, Andrea Righi wrote:
> On Tue, Oct 19, 2021 at 09:02:44AM -0300, Thadeu Lima de Souza Cascardo wrote:
> > On Tue, Oct 19, 2021 at 12:33:07PM +0200, Andrea Righi wrote:
> > > BugLink: https://bugs.launchpad.net/bugs/1947709
> > > 
> > > [Impact]
> > > 
> > > "UBUNTU: SAUCE: cachefiles: Page leaking in cachefiles_read_backing_file while
> > > vmscan is active" has been applied to fix a page leaking issue.
> > > 
> > > However a slightly different fix has been applied upstream:
> > > 
> > >  9a24ce5b66f9 ("cachefiles: Fix page leak in cachefiles_read_backing_file while vmscan is active")
> > > 
> > > Basically we are fixing the same issue in two different ways at the same time,
> > > but even worse our patch an introduce a potential NULL pointer dereference: we
> > > do a put_page(newpage) and set newpage = NULL in the main for() loop and then
> > > we may do additional put_page(newpage) after the main for loop if
> > > ret == -EEXIST, that would trigger the NULL pointer dereference.
> > > 
> > 
> > So, I see bionic has the SAUCE patch, but does not have the upstream one. We
> > should revert the SAUCE patch on bionic as well and apply the upstream patch
> > there.
> > 
> > Also, the upstream commit has a test case, are you able to use it?
> 
> Alright, I've been running the upstream commit test case for a while
> now, but I wasn't able to trigger any bug, so it's either a bad test
> case to trigger the bug that I see or my analysis about the potential
> NULL pointer dereference is incorrect. In any case I think it'd be still
> better to drop the SAUCE patch, because it's claiming to fix something
> that is already fixed by another upstream commit. Opinions?
> 

Yeah, though it would be great to be able to reproduce the potential failure,
the test also helps to exercise the code, which avoids surprising regressions.

Thanks, Andrea.
Cascardo.

Acked-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>

> Thanks,
> -Andrea