From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C19D3C433F5 for ; Tue, 1 Mar 2022 17:46:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236648AbiCARqy (ORCPT ); Tue, 1 Mar 2022 12:46:54 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40740 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231362AbiCARqw (ORCPT ); Tue, 1 Mar 2022 12:46:52 -0500 Received: from mail-pf1-x432.google.com (mail-pf1-x432.google.com [IPv6:2607:f8b0:4864:20::432]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9E2D061A13 for ; Tue, 1 Mar 2022 09:46:10 -0800 (PST) Received: by mail-pf1-x432.google.com with SMTP id y11so14927911pfa.6 for ; Tue, 01 Mar 2022 09:46:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=hW/YZmS7wpJS7lNeC9TLZr2YM2tyw8ZN9mG1toRHUEY=; b=n6SeFINnZOt1ieREkJkw7GKfq+9aKjZo1/dOXyhxGUtAm9PLy7rNDfN85E/cKEVcOh 1qJbKM2d4+FwBYEsjudLw2YwA+/gaRuopRoOzGC961IREwjWo7DHJRxCIsyFq/9/zH7H iU06/1PIlDpoPUhu44O1GYUzn+Y1ERDQ4hvLMa8CVD8XXRAWhAa7nOeC1uOqy5nNdhCd FD5hiuSRYZ1mVsECmjuoXAVG3jFIHx4DbO40wuuKWwgoHAs/be87O5alVV+56QkHQMpz OjgGz+fh4+boeUTAXspKSs83GBug9vmVjaPPs4RG7dkVdCRZj3Bw7CAqFUFf5IWOfJ6v uzLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=hW/YZmS7wpJS7lNeC9TLZr2YM2tyw8ZN9mG1toRHUEY=; b=4VdJACD7v94yN0s88qgg4En5MYI/7/JORIOrt749nBpCNvNk4VIohrNHaLMxJTotSU mipbzRoQj+LCbUtql4Rv5w14y7UjiKlMzB2qulAgRIdYlJRRD0v3itPMeUFir/0HfLhD ywIjJ0rkcGLaaKIfg/+iSfvu+R8lwqS4qZUloVN9dEzNiPPxHj50qyfSw3os2AVu8AEF la0tIj+ZObcxwbb0LWnSrnKpPwRk4mY1VlpQ6AYtzOu+3KQqzv+VFOJZuXpLcUN5rWnc Z+Mwe+HCxrVtDh06au1QSNgR5YeaEuYGeE+dZO3PZQ0AOqCm81VA9cbkzkIx9P2eq8w5 8u5Q== X-Gm-Message-State: AOAM533q5z3L3T0EGwAg9SDqvGDM9T2/KvrA7BLgVQ/+XmF3XpAe7H7p mtPPj7wAFf3krflawcI/PfBUPQ== X-Google-Smtp-Source: ABdhPJy29O9ruXlkZmz1FBD3hlywTLo4gG7yfSObUN32u+vZmDmQg1WYvsMi+/Is29GgqniD50TNfw== X-Received: by 2002:a05:6a00:a92:b0:4e0:57a7:2d5d with SMTP id b18-20020a056a000a9200b004e057a72d5dmr28694355pfl.81.1646156769845; Tue, 01 Mar 2022 09:46:09 -0800 (PST) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id me10-20020a17090b17ca00b001b9e6f62045sm2700322pjb.41.2022.03.01.09.46.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Mar 2022 09:46:09 -0800 (PST) Date: Tue, 1 Mar 2022 17:46:05 +0000 From: Sean Christopherson To: Maxim Levitsky Cc: kvm@vger.kernel.org, Jim Mattson , "H. Peter Anvin" , linux-kernel@vger.kernel.org, Vitaly Kuznetsov , Paolo Bonzini , Joerg Roedel , Thomas Gleixner , Dave Hansen , Wanpeng Li , Borislav Petkov , x86@kernel.org Subject: Re: [PATCH 4/4] KVM: x86: lapic: don't allow to set non default apic id when not using x2apic api Message-ID: References: <20220301135526.136554-1-mlevitsk@redhat.com> <20220301135526.136554-5-mlevitsk@redhat.com> <6f4819b4169bd4e2ca9ab710388ebd44b7918eed.camel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6f4819b4169bd4e2ca9ab710388ebd44b7918eed.camel@redhat.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 01, 2022, Maxim Levitsky wrote: > > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > > > index 80a2020c4db40..8d35f56c64020 100644 > > > --- a/arch/x86/kvm/lapic.c > > > +++ b/arch/x86/kvm/lapic.c > > > @@ -2618,15 +2618,14 @@ static int kvm_apic_state_fixup(struct kvm_vcpu *vcpu, > > > u32 *ldr = (u32 *)(s->regs + APIC_LDR); > > > u64 icr; > > > > > > - if (vcpu->kvm->arch.x2apic_format) { > > > - if (*id != vcpu->vcpu_id) > > > - return -EINVAL; > > > - } else { > > > - if (set) > > > - *id >>= 24; > > > - else > > > - *id <<= 24; > > > - } > > > + if (!vcpu->kvm->arch.x2apic_format && set) > > > + *id >>= 24; > > > + > > > + if (*id != vcpu->vcpu_id) > > > + return -EINVAL; > > > > This breaks backwards compability, userspace will start failing where it previously > > succeeded. It doesn't even require a malicious userspace, e.g. if userspace creates > > a vCPU with a vcpu_id > 255 vCPUs, the shift will truncate and cause failure. It'll > > obviously do weird things, but this code is 5.5 years old, I don't think it's worth > > trying to close a loophole that doesn't harm KVM. > > > > If we provide a way for userspace to opt into disallowiong APICID != vcpu_id, we > > can address this further upstream, e.g. reject vcpu_id > 255 without x2apic_format. > > This check is only when CPU is in x2apic mode. In this mode the X86 specs demands that > apic_id == vcpu_id. AMD's APM explicitly allows changing the x2APIC ID by writing the xAPIC ID and then switching to x2APIC mode. > When old API is used, APIC IDs are encoded in xapic format, even when vCPU is in x2apic > mode, meaning that apic id can't be more that 255 even in x2apic mode. Even on Intel CPUs, if userspace creates a vCPU with vcpu_id = 256, then the xAPIC ID will be (256 << 24) == 0. If userspace does get+set, then KVM will see 0 != 256 and reject the set with return -EINVAL. And as above, userspace that hasn't opted into the x2apic_format could also legitimately change the x2APIC ID. I 100% agree this is a mess and probably can't work without major shenanigans, but on the other hand this isn't harming anything, so why risk breaking userspace, even if the risk is infinitesimally small? I'm all for locking down the APIC ID with a userspace opt-in control, I just don't think it's worth trying to retroactively plug the various holes in KVM.