linux-kernel - Re: [PATCH] KVM: x86: Use gfn_to_pfn_cache for steal

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: <cd3fb0ac435d2516f9c5d921a3c8d0d09e3fbf73.camel@infradead.org>
Date: Fri, 02 Aug 2024 13:56:58 +0100
From: David Woodhouse <dwmw2@...radead.org>
To: Matthew Wilcox <willy@...radead.org>
Cc: Carsten Stollmaier <stollmc@...zon.com>, Sean Christopherson
 <seanjc@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>, Thomas Gleixner
 <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov
 <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
 "H. Peter Anvin" <hpa@...or.com>, nh-open-source@...zon.com, Peter Xu
 <peterx@...hat.com>,  Sebastian Biemueller <sbiemue@...zon.de>,
 kvm@...r.kernel.org, linux-kernel@...r.kernel.org, Andrew Morton
 <akpm@...ux-foundation.org>, "linux-mm@...ck.org" <linux-mm@...ck.org>,
 Andrea Arcangeli <aarcange@...hat.com>
Subject: Re: [PATCH] KVM: x86: Use gfn_to_pfn_cache for steal_time

On Fri, 2024-08-02 at 13:53 +0100, David Woodhouse wrote:
> On Fri, 2024-08-02 at 13:38 +0100, Matthew Wilcox wrote:
> > On Fri, Aug 02, 2024 at 01:03:16PM +0100, David Woodhouse wrote:
> > > On Fri, 2024-08-02 at 11:44 +0000, Carsten Stollmaier wrote:
> > > > handle_userfault uses TASK_INTERRUPTIBLE, so it is interruptible by
> > > > signals. do_user_addr_fault then busy-retries it if the pending signal
> > > > is non-fatal. This leads to contention of the mmap_lock.
> > 
> > Why does handle_userfault use TASK_INTERRUPTIBLE?  We really don't
> > want to stop handling a page fault just because somebody resized a
> > window or a timer went off.  TASK_KILLABLE, sure.
> 
> Well, the literal answer there in this case is "because we ask it to".
> 
> The handle_userfault() function will literally do what it's told by the
> fault flags: 
> 
> static inline unsigned int userfaultfd_get_blocking_state(unsigned int flags)
> {
>         if (flags & FAULT_FLAG_INTERRUPTIBLE)
>                 return TASK_INTERRUPTIBLE;
> 
>         if (flags & FAULT_FLAG_KILLABLE)
>                 return TASK_KILLABLE;
> 
>         return TASK_UNINTERRUPTIBLE;
> }
> 
> 
> Hence the other potential workaround I mentioned, for
> do_user_addr_fault() *not* to ask it to, for faults from the kernel:
> 
> > > 
> > > --- a/arch/x86/mm/fault.c
> > > +++ b/arch/x86/mm/fault.c
> > > @@ -1304,6 +1304,8 @@ void do_user_addr_fault(struct pt_regs *regs,
> > >          */
> > >         if (user_mode(regs))
> > >                 flags |= FAULT_FLAG_USER;
> > > +       else
> > > +               flags &= ~FAULT_FLAG_INTERRUPTIBLE;
> > >  
> > >  #ifdef CONFIG_X86_64
> > >         /*
> > > 
> 
> 
> But I don't know that I agree with your statement above, that we "don't
> want to stop handling a page fault just because somebody resized a
> window or a timer went off". 

See also "we don't want to stop waiting for a page fault, just because
somebody hit Ctrl-C, but SIGINT has a trivial handler to do some minor
cleanup before exiting so it isn't considered a *fatal* signal".

I'm very sure I'd disagree with that one :)

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