linux-kernel - Re: [PATCH 05/28] mm, swap: sanitize swap cache lookup convention

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: <CAMgjq7BpfueOn9ms8apRX-6dF8rZGtbC=MuZzSD7hbZxtw=Kdg@mail.gmail.com>
Date: Tue, 20 May 2025 11:31:25 +0800
From: Kairui Song <ryncsn@...il.com>
To: Barry Song <21cnbao@...il.com>
Cc: akpm@...ux-foundation.org, baohua@...nel.org, 
	baolin.wang@...ux.alibaba.com, bhe@...hat.com, chrisl@...nel.org, 
	david@...hat.com, hannes@...xchg.org, hughd@...gle.com, 
	kaleshsingh@...gle.com, linux-kernel@...r.kernel.org, linux-mm@...ck.org, 
	nphamcs@...il.com, ryan.roberts@....com, shikemeng@...weicloud.com, 
	tim.c.chen@...ux.intel.com, willy@...radead.org, ying.huang@...ux.alibaba.com, 
	yosryahmed@...gle.com
Subject: Re: [PATCH 05/28] mm, swap: sanitize swap cache lookup convention

On Mon, May 19, 2025 at 12:38 PM Barry Song <21cnbao@...il.com> wrote:
>
> > From: Kairui Song <kasong@...cent.com>
>
> > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> > index e5a0db7f3331..5b4f01aecf35 100644
> > --- a/mm/userfaultfd.c
> > +++ b/mm/userfaultfd.c
> > @@ -1409,6 +1409,10 @@ static int move_pages_pte(struct mm_struct *mm, pmd_t *dst_pmd, pmd_t *src_pmd,
> >                               goto retry;
> >                       }
> >               }
> > +             if (!folio_swap_contains(src_folio, entry)) {
> > +                     err = -EBUSY;
> > +                     goto out;
> > +             }
>
> It seems we don't need this. In move_swap_pte(), we have been checking pte pages
> are stable:
>
>         if (!is_pte_pages_stable(dst_pte, src_pte, orig_dst_pte, orig_src_pte,
>                                  dst_pmd, dst_pmdval)) {
>                 double_pt_unlock(dst_ptl, src_ptl);
>                 return -EAGAIN;
>         }

The tricky part is when swap_cache_get_folio returns the folio, both
folio and ptes are unlocked. So is it possible that someone else
swapped in the entries, then swapped them out again using the same
entries?

The folio will be different here but PTEs are still the same value to
they will pass the is_pte_pages_stable check, we previously saw
similar races with anon fault or shmem. I think more strict checking
won't hurt here.

>
> Also, -EBUSY is somehow incorrect error code.

Yes, thanks, I'll use EAGAIN here just like move_swap_pte.


>
> >               err = move_swap_pte(mm, dst_vma, dst_addr, src_addr, dst_pte, src_pte,
> >                               orig_dst_pte, orig_src_pte, dst_pmd, dst_pmdval,
> >                               dst_ptl, src_ptl, src_folio);
> >
>
> Thanks
> Barry