Messages in this thread | | | Date | Thu, 3 Jun 2021 20:26:02 -0700 (PDT) | From | Hugh Dickins <> | Subject | Re: [PATCH] mm/swap: Fix pte_same_as_swp() not removing uffd-wp bit when compare |
| |
On Thu, 3 Jun 2021, Peter Xu wrote:
> I found it by pure code review, that pte_same_as_swp() of unuse_vma() didn't
Yes, that is an odd corner, easily missed.
> take uffd-wp bit into account when comparing ptes. pte_same_as_swp() returning > false negative could cause failure to swapoff swap ptes that was wr-protected > by userfaultfd. > > Cc: Hugh Dickins <hughd@google.com> > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Peter Xu <peterx@redhat.com>
I expect you're right: swapoff used to hang forever (but interruptibly) when this went wrong on powerpc originally. I don't know the uffd_wp (nor the soft_dirty) end of it, but treating uffd_wp and soft_dirty together looks a very good approach, so I'll venture an
Acked-by: Hugh Dickins <hughd@google.com>
But I think it should have a uffd_wp Fixes tag and be Cc stable.
> --- > include/linux/swapops.h | 15 +++++++++++---- > mm/swapfile.c | 2 +- > 2 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/include/linux/swapops.h b/include/linux/swapops.h > index af3d2661e41e..d356ab4047f7 100644 > --- a/include/linux/swapops.h > +++ b/include/linux/swapops.h > @@ -23,6 +23,16 @@ > #define SWP_TYPE_SHIFT (BITS_PER_XA_VALUE - MAX_SWAPFILES_SHIFT) > #define SWP_OFFSET_MASK ((1UL << SWP_TYPE_SHIFT) - 1) > > +/* Clear all flags but only keep swp_entry_t related information */ > +static inline pte_t pte_swp_clear_flags(pte_t pte) > +{ > + if (pte_swp_soft_dirty(pte)) > + pte = pte_swp_clear_soft_dirty(pte); > + if (pte_swp_uffd_wp(pte)) > + pte = pte_swp_clear_uffd_wp(pte); > + return pte; > +} > + > /* > * Store a type+offset into a swp_entry_t in an arch-independent format > */ > @@ -66,10 +76,7 @@ static inline swp_entry_t pte_to_swp_entry(pte_t pte) > { > swp_entry_t arch_entry; > > - if (pte_swp_soft_dirty(pte)) > - pte = pte_swp_clear_soft_dirty(pte); > - if (pte_swp_uffd_wp(pte)) > - pte = pte_swp_clear_uffd_wp(pte); > + pte = pte_swp_clear_flags(pte); > arch_entry = __pte_to_swp_entry(pte); > return swp_entry(__swp_type(arch_entry), __swp_offset(arch_entry)); > } > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 6212bc033602..1e07d1c776f2 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -1887,7 +1887,7 @@ unsigned int count_swap_pages(int type, int free) > > static inline int pte_same_as_swp(pte_t pte, pte_t swp_pte) > { > - return pte_same(pte_swp_clear_soft_dirty(pte), swp_pte); > + return pte_same(pte_swp_clear_flags(pte), swp_pte); > } > > /* > -- > 2.31.1
| |