[PATCH] mm: Fix list corruption in put_pages_list

[PATCH] mm: Fix list corruption in put_pages_list

[Matthew Wilcox (Oracle)]

My recent change to put_pages_list() dereferences folio->lru.next after
returning the folio to the page allocator.  Usually this is now on the
pcp list with other free folios, so we try to free an already-free
folio.  This only happens with lists that have more than 15 entries, so
it wasn't immediately discovered.  Revert to using list_for_each_safe()
so we dereference lru.next before disposing of the folio.

Reported-by: "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>
Fixes: 24835f899c01 (mm: use free_unref_folios() in put_pages_list())
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/swap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/swap.c b/mm/swap.c
index a910af21ba68..1d4b7713605d 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -139,10 +139,10 @@ EXPORT_SYMBOL(__folio_put);
 void put_pages_list(struct list_head *pages)
 {
 	struct folio_batch fbatch;
-	struct folio *folio;
+	struct folio *folio, *next;
 
 	folio_batch_init(&fbatch);
-	list_for_each_entry(folio, pages, lru) {
+	list_for_each_entry_safe(folio, next, pages, lru) {
 		if (!folio_put_testzero(folio))
 			continue;
 		if (folio_test_hugetlb(folio)) {
-- 
2.43.0

Re: [PATCH] mm: Fix list corruption in put_pages_list

[Andrew Morton]

On Wed,  6 Mar 2024 21:27:30 +0000 "Matthew Wilcox (Oracle)" <willy@infradead.org> wrote:

> My recent change to put_pages_list() dereferences folio->lru.next after
> returning the folio to the page allocator.  Usually this is now on the
> pcp list with other free folios, so we try to free an already-free
> folio.  This only happens with lists that have more than 15 entries, so
> it wasn't immediately discovered.  Revert to using list_for_each_safe()
> so we dereference lru.next before disposing of the folio.
> 
> Reported-by: "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>

I'm unable to find the bug report on linux-mm.  Help please?

> Fixes: 24835f899c01 (mm: use free_unref_folios() in put_pages_list())
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/swap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/swap.c b/mm/swap.c
> index a910af21ba68..1d4b7713605d 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -139,10 +139,10 @@ EXPORT_SYMBOL(__folio_put);
>  void put_pages_list(struct list_head *pages)
>  {
>  	struct folio_batch fbatch;
> -	struct folio *folio;
> +	struct folio *folio, *next;
>  
>  	folio_batch_init(&fbatch);
> -	list_for_each_entry(folio, pages, lru) {
> +	list_for_each_entry_safe(folio, next, pages, lru) {
>  		if (!folio_put_testzero(folio))
>  			continue;
>  		if (folio_test_hugetlb(folio)) {
> -- 
> 2.43.0

Re: [PATCH] mm: Fix list corruption in put_pages_list

[Matthew Wilcox]

On Wed, Mar 06, 2024 at 01:45:57PM -0800, Andrew Morton wrote:
> On Wed,  6 Mar 2024 21:27:30 +0000 "Matthew Wilcox (Oracle)" <willy@infradead.org> wrote:
> 
> > My recent change to put_pages_list() dereferences folio->lru.next after
> > returning the folio to the page allocator.  Usually this is now on the
> > pcp list with other free folios, so we try to free an already-free
> > folio.  This only happens with lists that have more than 15 entries, so
> > it wasn't immediately discovered.  Revert to using list_for_each_safe()
> > so we dereference lru.next before disposing of the folio.
> > 
> > Reported-by: "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>
> 
> I'm unable to find the bug report on linux-mm.  Help please?

https://lore.kernel.org/intel-gfx/SJ1PR11MB61292145F3B79DA58ADDDA63B9232@SJ1PR11MB6129.namprd11.prod.outlook.com/

(wasn't cc'd to linux-mm)

Re: [PATCH] mm: Fix list corruption in put_pages_list

[Andrew Morton]

On Wed, 6 Mar 2024 21:47:35 +0000 Matthew Wilcox <willy@infradead.org> wrote:

> On Wed, Mar 06, 2024 at 01:45:57PM -0800, Andrew Morton wrote:
> > On Wed,  6 Mar 2024 21:27:30 +0000 "Matthew Wilcox (Oracle)" <willy@infradead.org> wrote:
> > 
> > > My recent change to put_pages_list() dereferences folio->lru.next after
> > > returning the folio to the page allocator.  Usually this is now on the
> > > pcp list with other free folios, so we try to free an already-free
> > > folio.  This only happens with lists that have more than 15 entries, so
> > > it wasn't immediately discovered.  Revert to using list_for_each_safe()
> > > so we dereference lru.next before disposing of the folio.
> > > 
> > > Reported-by: "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>
> > 
> > I'm unable to find the bug report on linux-mm.  Help please?
> 
> https://lore.kernel.org/intel-gfx/SJ1PR11MB61292145F3B79DA58ADDDA63B9232@SJ1PR11MB6129.namprd11.prod.outlook.com/
> 
> (wasn't cc'd to linux-mm)

Ah, OK, thanks.  I'll add that as a Closes: thingy.