linux-kernel - Re: [PATCH] mm: gup: fix infinite loop within __get_longterm

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGWkznEWmJU9q8d2x29HSU9wU3ue6MdaM1tVn-u=OiYOW5M9Qg@mail.gmail.com>
Date: Tue, 21 Jan 2025 09:28:49 +0800
From: Zhaoyang Huang <huangzhaoyang@...il.com>
To: John Hubbard <jhubbard@...dia.com>
Cc: "zhaoyang.huang" <zhaoyang.huang@...soc.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Alistair Popple <apopple@...dia.com>, linux-mm@...ck.org, linux-kernel@...r.kernel.org, 
	steve.kang@...soc.com
Subject: Re: [PATCH] mm: gup: fix infinite loop within __get_longterm_locked

On Tue, Jan 21, 2025 at 3:34 AM John Hubbard <jhubbard@...dia.com> wrote:
>
> On 1/20/25 1:26 AM, zhaoyang.huang wrote:
> > From: Zhaoyang Huang <zhaoyang.huang@...soc.com>
> >
> > Infinite loop within __get_longterm_locked detected in an unique usage
> > of pin_user_pages where the VA's pages are all unpinnable(vm_ops->fault
> > function allocate pages via cma_alloc for hardware purpose and leave them
> > out of LRU). Fixing this by have 'collected' reflect the actual number
> > of pages in movable_folio_list.
>
> The above is rather terse, although perhaps by kernel standards it's OK.
>
> Isn't this missing a Fixes tag?
>
> Fixes: 67e139b02d994 ("mm/gup.c: refactor
> check_and_migrate_movable_pages()")
ok. will amend in v2
>
> >
> > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@...soc.com>
> > ---
> >   mm/gup.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/gup.c b/mm/gup.c
> > index 3b75e631f369..2231ce7221f9 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -2341,8 +2341,6 @@ static unsigned long collect_longterm_unpinnable_folios(
> >               if (folio_is_longterm_pinnable(folio))
> >                       continue;
> >
> > -             collected++;
> > -
> >               if (folio_is_device_coherent(folio))
> >                       continue;
> >
> > @@ -2359,6 +2357,8 @@ static unsigned long collect_longterm_unpinnable_folios(
> >               if (!folio_isolate_lru(folio))
> >                       continue;
> >
> > +             collected++;
> > +
>
> Well, this seems correct to me. Somehow I talked myself into believing
> that it was OK to do collected++ early, even though later on we skip
> actually collecting the folio, thus miscounting things.
>
> But now I believe it was just incorrect all along.
>
>
> Reviewed-by: John Hubbard <jhubbard@...dia.com>
thanks
>
> thanks,
> --
> John Hubbard