linux-kernel - Re: [PATCH v6 06/38] mm: Add default definition of set

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

Message-ID: <bb0e7a9f016f22993303dd10c2c92a6ef3c7ebf6.camel@infradead.org>
Date:   Thu, 12 Oct 2023 15:43:54 +0100
From:   David Woodhouse <dwmw2@...radead.org>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-arch@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Mike Rapoport <rppt@...nel.org>,
        xen-devel <xen-devel@...ts.xenproject.org>
Subject: Re: [PATCH v6 06/38] mm: Add default definition of set_ptes()

On Thu, 2023-10-12 at 15:05 +0100, Matthew Wilcox wrote:
> On Thu, Oct 12, 2023 at 02:53:05PM +0100, David Woodhouse wrote:
> > > +       arch_enter_lazy_mmu_mode();
> > > +       for (;;) {
> > > +               set_pte(ptep, pte);
> > > +               if (--nr == 0)
> > > +                       break;
> > > +               ptep++;
> > > +               pte = __pte(pte_val(pte) + (1UL << PFN_PTE_SHIFT));
> > > +       }
> > > +       arch_leave_lazy_mmu_mode();
> > 
> > This breaks the Xen PV guest.
> > 
> > In move_ptes() in mm/mremap.c we arch_enter_lazy_mmu_mode() and then
> > loop calling set_pte_at(). Which now (or at least in a few commits time
> > when you wire it up for x86 in commit a3e1c9372c9b959) ends up in your
> > implementation of set_ptes(), calls arch_enter_lazy_mmu_mode() again,
> > and:
> > 
> > [    0.628700] ------------[ cut here ]------------
> > [    0.628718] kernel BUG at arch/x86/kernel/paravirt.c:144!
> 
> Easy fix ... don't do that ;-)
> 
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index af7639c3b0a3..f3da8836f689 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -231,9 +231,11 @@ static inline pte_t pte_next_pfn(pte_t pte)
>  static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
>                 pte_t *ptep, pte_t pte, unsigned int nr)
>  {
> +       bool multiple = nr > 1;
>         page_table_check_ptes_set(mm, ptep, pte, nr);
>  
> -       arch_enter_lazy_mmu_mode();
> +       if (multiple)
> +               arch_enter_lazy_mmu_mode();
>         for (;;) {
>                 set_pte(ptep, pte);
>                 if (--nr == 0)
> @@ -241,7 +243,8 @@ static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
>                 ptep++;
>                 pte = pte_next_pfn(pte);
>         }
> -       arch_leave_lazy_mmu_mode();
> +       if (multiple)
> +               arch_leave_lazy_mmu_mode();
>  }
>  #endif
>  #define set_pte_at(mm, addr, ptep, pte) set_ptes(mm, addr, ptep, pte, 1)
> 
> I think long-term, we should make lazy_mmu_mode nestable.  But this is
> a reasonable quick fix.

I don't much like doing it implicitly based on (nr==1) but sure, as a
quick fix that works. The 64-bit PV guest now boots again.

Tested-by: David Woodhouse <dwmw@...zon.co.uk>

Thanks.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)