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 D37A6E9270D for ; Thu, 5 Oct 2023 15:53:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234779AbjJEPxb (ORCPT ); Thu, 5 Oct 2023 11:53:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49838 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232953AbjJEPv2 (ORCPT ); Thu, 5 Oct 2023 11:51:28 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 282774DF63 for ; Thu, 5 Oct 2023 07:02:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1696514416; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/r6/vxDZ7SlHr2MUNh2FMyN5pOqeM6f7KWklCXeGbBg=; b=hQxz9iUoKr24M6Pziq/5HfgHT1KWoefDsfpw7MbK+M+A2tOPoWE4x+j1OOE9HW5nl6xraO rFAfsnPcdsMMF4rs1PDav7ybDb61hBdXYycB9TP8RCwwhzkD+pSkWE8BMP+z1Kp7/okDyW JwgZ4ixzUPONA4iEQ5Bu1qpbzFLoPXY= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-516-9DSwtSxtOe2DBJXPh0G_HQ-1; Thu, 05 Oct 2023 08:58:33 -0400 X-MC-Unique: 9DSwtSxtOe2DBJXPh0G_HQ-1 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-40540179bcdso5961455e9.2 for ; Thu, 05 Oct 2023 05:58:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696510712; x=1697115512; h=content-transfer-encoding:mime-version:user-agent:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=/r6/vxDZ7SlHr2MUNh2FMyN5pOqeM6f7KWklCXeGbBg=; b=Bg/4LBhCEJF52FRpgXS2DpLulNF4ViQKAUyCI/XuOd9z7YCVqIwuUKfsqlX2Uo1jP+ 05xlfktCbR0j0on8wbyYgcYotp72VoFfxGxdGbs8RHxc3QRQOXaZIpy4L0eFF1K84WTx eV5DP8hmiObDqd7A6G8l53Y+yvvUvTEckNw3q0iGTSOBkKrRE2ul1UMCMFJAvxOcNJCC NCe3BnUReCRD4r5nCtc2BM+EkHDWuSYGkqqJnLEgdUv4OaMtXpbXZtS4gdfl+j1cZBTx x9rSqhPoRiXgVse6Tcw6vYRsWhDNCSzs0dsv73NXjblULQ7IagTiSay1Cgnsx1E8ufDD /a0Q== X-Gm-Message-State: AOJu0YzHgZbeUVfunK071+DMENd3ntCROxmnU9UYbKZCds3Jl3vs+oHZ ETJPEfU68QbVwtwjVz9l4fCWxreweTdOMWKwIvNZy1RH+NY/afMsYeAUcRBFC0xWAoiTfX19AgB iXb5HdwEbcHiL6cDM8X4W4G2t X-Received: by 2002:a1c:7914:0:b0:401:1b58:72f7 with SMTP id l20-20020a1c7914000000b004011b5872f7mr4896064wme.38.1696510712648; Thu, 05 Oct 2023 05:58:32 -0700 (PDT) X-Google-Smtp-Source: AGHT+IH7lzNVbgVxPFaaWui24YMcE2z6WO73JBOPVtj5jYlX2YjQWj64rpJKORnSmCPW9jT0XGo+RA== X-Received: by 2002:a1c:7914:0:b0:401:1b58:72f7 with SMTP id l20-20020a1c7914000000b004011b5872f7mr4896043wme.38.1696510712176; Thu, 05 Oct 2023 05:58:32 -0700 (PDT) Received: from starship ([89.237.100.246]) by smtp.gmail.com with ESMTPSA id y24-20020a7bcd98000000b004064741f855sm1453766wmj.47.2023.10.05.05.58.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Oct 2023 05:58:31 -0700 (PDT) Message-ID: Subject: Re: [PATCH 09/10] KVM: SVM: Drop redundant check in AVIC code on ID during vCPU creation From: Maxim Levitsky To: Sean Christopherson , Paolo Bonzini , Joerg Roedel Cc: kvm@vger.kernel.org, iommu@lists.linux.dev, linux-kernel@vger.kernel.org Date: Thu, 05 Oct 2023 15:58:30 +0300 In-Reply-To: <20230815213533.548732-10-seanjc@google.com> References: <20230815213533.548732-1-seanjc@google.com> <20230815213533.548732-10-seanjc@google.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5 (3.36.5-2.fc32) MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org У вт, 2023-08-15 у 14:35 -0700, Sean Christopherson пише: > Drop avic_get_physical_id_entry()'s compatibility check on the incoming > ID, as its sole caller, avic_init_backing_page(), performs the exact same > check. Drop avic_get_physical_id_entry() entirely as the only remaining > functionality is getting the address of the Physical ID table, and > accessing the array without an immediate bounds check is kludgy. > > No functional change intended. > > Signed-off-by: Sean Christopherson > --- > arch/x86/kvm/svm/avic.c | 28 ++++++---------------------- > 1 file changed, 6 insertions(+), 22 deletions(-) > > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index 3b2d00d9ca9b..6803e2d7bc22 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -263,26 +263,12 @@ void avic_init_vmcb(struct vcpu_svm *svm, struct vmcb *vmcb) > avic_deactivate_vmcb(svm); > } > > -static u64 *avic_get_physical_id_entry(struct kvm_vcpu *vcpu, > - unsigned int index) > -{ > - u64 *avic_physical_id_table; > - struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm); > - > - if ((!x2avic_enabled && index > AVIC_MAX_PHYSICAL_ID) || > - (index > X2AVIC_MAX_PHYSICAL_ID)) > - return NULL; While removing this code doesn't introduce a bug, it does make it less safe, because new code just blindly trusts that vcpu_id will never be out of bounds of the physical id table. Bugs happen and that can and will someday happen. > - > - avic_physical_id_table = page_address(kvm_svm->avic_physical_id_table_page); > - > - return &avic_physical_id_table[index]; > -} > - > static int avic_init_backing_page(struct kvm_vcpu *vcpu) > { > - u64 *entry, new_entry; > + struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm); > + struct vcpu_svm *svm = to_svm(vcpu); > + u64 *table, new_entry; > int id = vcpu->vcpu_id; > - struct vcpu_svm *svm = to_svm(vcpu); > > /* > * Inhibit AVIC if the vCPU ID is bigger than what is supported by AVIC > @@ -318,15 +304,13 @@ static int avic_init_backing_page(struct kvm_vcpu *vcpu) > } > > /* Setting AVIC backing page address in the phy APIC ID table */ > - entry = avic_get_physical_id_entry(vcpu, id); > - if (!entry) > - return -EINVAL; > + table = page_address(kvm_svm->avic_physical_id_table_page); > > new_entry = avic_get_backing_page_address(svm) | > AVIC_PHYSICAL_ID_ENTRY_VALID_MASK; > - WRITE_ONCE(*entry, new_entry); Here I prefer to at least have an assert that id is in bounds of a page (at least less than 512) so that a bug will not turn into a security issue by overflowing the buffer. > + WRITE_ONCE(table[id], new_entry); > > - svm->avic_physical_id_cache = entry; > + svm->avic_physical_id_cache = &table[id]; > > return 0; > } Best regards, Maxim Levitsky