linux-kernel - Re: [PATCH v4] KVM: x86/tsc: Don't sync user changes to TSC with KVM-initiated change

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]

Date:   Wed, 13 Sep 2023 16:41:07 +0800
From:   Like Xu <like.xu.linux@...il.com>
To:     David Woodhouse <dwmw2@...radead.org>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Oliver Upton <oliver.upton@...ux.dev>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Sean Christopherson <seanjc@...gle.com>
Subject: Re: [PATCH v4] KVM: x86/tsc: Don't sync user changes to TSC with
 KVM-initiated change

On 13/9/2023 4:10 pm, David Woodhouse wrote:
> On Fri, 2023-08-11 at 15:59 -0700, Sean Christopherson wrote:
>> The problem isn't that the sync code doesn't differentiate between kernel and
>> user-initiated writes, because parts of the code *do* differentiate.  I think it's
>> more accurate to say that the problem is that the sync code doesn't differentiate
>> between userspace initializing the TSC and userspace attempting to synchronize the
>> TSC.
> 
> I'm not utterly sure that *I* differentiate between userspace
> "initializing the TSC" and attempting to "synchronize the TSC". What
> *is* the difference?

I'd be more inclined to Oliver's explanation in this version of the changelog
that different tsc_offsets are used to calculate guest_tsc value between the vcpu
is created and when it is first set by usersapce. This extra synchronization is not
expected for guest based on user's bugzilla report.

> 
> Userspace is merely *setting* the TSC for a given vCPU, regardless of
> whether other vCPUs even exist.
> 
> But we have to work around the fundamental brokenness of the legacy
> API, whose semantics are most accurately described as "Please set the
> TSC to precisely <x> because that's what it should have been *some*
> time around now, if I wasn't preempted very much between when I
> calculated it and when you see this ioctl".
> 
> That's why — for the legacy API only — we have this hack to make the
> TSCs *actually* in sync if they're close. Because without it, there;s
> *no* way the VMM can restore a guest with its TSCs actually in sync.
> 
> I think the best answer to the bug report that led to this patch is
> just "Don't use the legacy API then". Use KVM_VCPU_TSC_OFFSET which is
> defined as "the TSC was <x> at KVM time <y>" and is actually *sane*.
> 

Two hands in favor. Using the new KVM_VCPU_TSC_OFFSET API and a little
fix on the legacy API is not conflict. Thank you for reviewing it.