linux-kernel - Re: [PATCH v2] entry/kvm: Make vCPU tasks exit to userspace when a livepatch is pending

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: <20220504145732.GD8069@pathway.suse.cz>
Date:   Wed, 4 May 2022 16:57:32 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Seth Forshee <sforshee@...italocean.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Andy Lutomirski <luto@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Jiri Kosina <jikos@...nel.org>,
        Miroslav Benes <mbenes@...e.cz>,
        Paolo Bonzini <pbonzini@...hat.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Jens Axboe <axboe@...nel.dk>,
        Sean Christopherson <seanjc@...gle.com>,
        linux-kernel@...r.kernel.org, live-patching@...r.kernel.org,
        kvm@...r.kernel.org
Subject: Re: [PATCH v2] entry/kvm: Make vCPU tasks exit to userspace when a
 livepatch is pending

On Wed 2022-05-04 09:44:07, Seth Forshee wrote:
> On Wed, May 04, 2022 at 04:28:09PM +0200, Petr Mladek wrote:
> > On Wed 2022-05-04 08:50:22, Seth Forshee wrote:
> > > On Wed, May 04, 2022 at 03:07:53PM +0200, Petr Mladek wrote:
> > > > On Tue 2022-05-03 12:49:34, Seth Forshee wrote:
> > > > > A task can be livepatched only when it is sleeping or it exits to
> > > > > userspace. This may happen infrequently for a heavily loaded vCPU task,
> > > > > leading to livepatch transition failures.
> > > > 
> > > > The problem was solved by sending a fake signal, see the commit
> > > > 0b3d52790e1cfd6b80b826 ("livepatch: Remove signal sysfs attribute").
> > > > It was achieved by calling signal_wake_up(). It set TIF_SIGPENDING
> > > > and woke the task. It interrupted the syscall and the task was
> > > > transitioned when leaving to the userspace.
> > > > 
> > > > signal_wake_up() was later replaced by set_notify_signal(),
> > > > see the commit 8df1947c71ee53c7e21 ("livepatch: Replace
> > > > the fake signal sending with TIF_NOTIFY_SIGNAL infrastructure").
> > > > The difference is that set_notify_signal() uses TIF_NOTIFY_SIGNAL
> > > > instead of TIF_SIGPENDING.
> > > > 
> > > > The effect is the same when running on a real hardware. The syscall
> > > > gets interrupted and exit_to_user_mode_loop() is called where
> > > > the livepatch state is updated (task migrated).
> > > > 
> > > > But it works a different way in kvm where the task works are
> > > > called in the guest mode and the task does not return into
> > > > the user space in the host mode.
> > > 
> > > > > --- a/kernel/entry/kvm.c
> > > > > +++ b/kernel/entry/kvm.c
> > > > > @@ -14,7 +14,12 @@ static int xfer_to_guest_mode_work(struct kvm_vcpu *vcpu, unsigned long ti_work)
> > > > >  				task_work_run();
> > > > >  		}
> > > > >  
> > > > > -		if (ti_work & _TIF_SIGPENDING) {
> > > > > +		/*
> > > > > +		 * When a livepatch is pending, force an exit to userspace
> > > > > +		 * as though a signal is pending to allow the task to be
> > > > > +		 * patched.
> > > > > +		 */
> > > > > +		if (ti_work & (_TIF_SIGPENDING | _TIF_PATCH_PENDING)) {
> > > > >  			kvm_handle_signal_exit(vcpu);
> > 
> > Another problem. Is it safe to call kvm_handle_signal_exit(vcpu)
> > for kthreads?
> > 
> > kthreads have _TIF_PATCH_PENDING when they need the livepatch transition.
> > But kthreads never leave kernel so we do not send the fake signal
> > signals to them.
> 
> xfer_to_guest_mode_handle_work() should only be getting called on user
> threads running ioctl(KVM_RUN).

Great!

> > In this case, we should revert the commit 8df1947c71ee53c7e21
> > ("livepatch: Replace the fake signal sending with TIF_NOTIFY_SIGNAL
> > infrastructure"). The flag TIF_NOTIFY_SIGNAL clearly does not guarantee
> > restarting the syscall or exiting to the user space with -EINTR.
> > 
> > It should solve this problem. And it looks like a cleaner solution
> > to me.
> 
> It looks like that should fix the issue. I'll test to confirm.

Even better solution would be what Eric suggested, see
https://lore.kernel.org/r/87r159fkmp.fsf@email.froward.int.ebiederm.org

But we need to make sure that the syscall really gets restarted
when the livepatch state is updated.

Best Regards,
Petr