From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C84161E3787 for ; Mon, 20 Jan 2025 14:20:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737382858; cv=none; b=hHkhgS7MqOzJqFbTDK4xMnIQ4go1f7YGqA6eLnJHgB4dRFzcPi4WbvuOk5WrrR+Ev/ZLKP/7cet8b6syWTWyNxErKA9MYNV2rHLKeNN3Ac0AWH2pOmBMjtZ4AiVmjRV3U8WLCemJDbK7a9VgS761aconnAH5xa1u4oyGYrtD1LE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737382858; c=relaxed/simple; bh=suKxKDTEjcS+hBYZlJUZIHQsfCWIJB64PESfiLzeAyQ=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=HhW2H/dK5gluOpcwh6IRLaO5k4gciLm1d8gFHj7le+Sd0kpQHR+s40L+6gjDvRfAqA+XBMsGMKDE6I9tzKfermtcFrheokv/U6vJYwZ0DizQooD8bbF2/x5osSfRPXMtO1LVcwnBCuZNcBFO8Zf4X/HB/axMVNSEEzrm0SXJatI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=PiNPAPFm; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="PiNPAPFm" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1737382854; 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: in-reply-to:in-reply-to:references:references; bh=SvN2HL1s3vl0r88ePE5HTVvwon5QIhXjCIcs6kwnVUI=; b=PiNPAPFmkV8rrglV8t7IvoxHDLTElB5WuHqSWz44lzYHGKTrzoq3FopVeAlPCrFLIp8OoK nq8GRdjvJD7e8tAsSBe82yjqtY57wi41JPJ1pe/6KNpCmwvylcV4tNYVu5dHFbBX0WavGy XU2Y1WOzW25bslBOfqvvp09FK67mfGY= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-633-pY7jfjNlOBWxobZDB8Xjgg-1; Mon, 20 Jan 2025 09:20:53 -0500 X-MC-Unique: pY7jfjNlOBWxobZDB8Xjgg-1 X-Mimecast-MFC-AGG-ID: pY7jfjNlOBWxobZDB8Xjgg Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-38634103b0dso2820875f8f.2 for ; Mon, 20 Jan 2025 06:20:53 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737382852; x=1737987652; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=SvN2HL1s3vl0r88ePE5HTVvwon5QIhXjCIcs6kwnVUI=; b=TAfNbAcOohh+3I1NxnsI1Qq3LNA4Ia2jfb/4qiDZ2V4Ejd0Z2Mmvv6+p3I0AEm0TXD PA+13BpCECX730dNIyPyEpT7YeFGF2PW9/TCSUgCtPEi46xLBL91EywyKjqJmTN2Ogcd 6APUZalrO8LfNG21AGlApzcmDdVj9c8GKG7wl1O9AXTJStmoeBAvTGdDyEWm+osJQSIj YCqfQWqGhpWsD+gpfJ+FxIaoRCGnDVvh16hEvIknGC3BYK0TL14BKUWm3+oAcukVf8xM w01Bwm8Gz1rhifyyg3tA+C/JeNCA/4aZay1Y0gFsCs4ITc1zlkFbgEBb6fhXoIhj+4Hj XzHA== X-Forwarded-Encrypted: i=1; AJvYcCV3M1fGmm5ogfKoDUZ1gIMZSBR9OhBd32xAVg+I3aqlUMc4lSKO+myXNtvJYiqscEgMnEUdJfKNYpWEiv4=@vger.kernel.org X-Gm-Message-State: AOJu0YwjA5B3iDAC28tEAJCCjR1X0emeDMdzmVDmFoQbtla5v0MPs/L5 53YOdGTKZ+NPvSDjA2x7olIhdYx69PdDqtCi2MZ2gXDhLz8h4ThiYoMxd7OrmGM9TP03Jseycgu 7zI0pcgLtSO0iwH3FIFn+At5orNQZ2GZWxBweHitapPEibVTJgtcXhjwkyKKJjQ== X-Gm-Gg: ASbGncu448zizxMaNQeJ9vjsXtrP5s+Lo+7/ZbQirDuDcZInbbIVDCR/9/FIYw0bwH9 YBid3Q/H4Dhz0ckGx8iKIdC5obDWsxSbdEw+lYgcbDeCmRfParOpNSoVPzc1ZyDnu0u0sz8LSe8 6HAqDBy7ufP8qP+VfCWj+5Z8m1orXNy5CgbUitXw5EWOSYLTK60v++iuXCMbEu9owi1XX7Sta3p EtUihZGB9bsJBBkiSz+uNqrMbfTjvit6U/Fy5exOrYwdj0K/r06QAJEb5wVvH1y X-Received: by 2002:a5d:47c2:0:b0:385:e8ce:7483 with SMTP id ffacd0b85a97d-38bf56555admr9436274f8f.4.1737382852041; Mon, 20 Jan 2025 06:20:52 -0800 (PST) X-Google-Smtp-Source: AGHT+IEQRZWBg3BiN14B3lboMp8g8k3LhBlXdMxIMG0O7SHp8HTkJQ4e3YMeYry8CkbCXZokHWp2Gg== X-Received: by 2002:a5d:47c2:0:b0:385:e8ce:7483 with SMTP id ffacd0b85a97d-38bf56555admr9436260f8f.4.1737382851631; Mon, 20 Jan 2025 06:20:51 -0800 (PST) Received: from fedora (g2.ign.cz. [91.219.240.8]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-438904625f5sm144155115e9.28.2025.01.20.06.20.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Jan 2025 06:20:51 -0800 (PST) From: Vitaly Kuznetsov To: Sean Christopherson , Paolo Bonzini Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Dongjie Zou Subject: Re: [PATCH v2 3/4] KVM: selftests: Manage CPUID array in Hyper-V CPUID test's core helper In-Reply-To: <20250118003454.2619573-4-seanjc@google.com> References: <20250118003454.2619573-1-seanjc@google.com> <20250118003454.2619573-4-seanjc@google.com> Date: Mon, 20 Jan 2025 15:20:50 +0100 Message-ID: <875xm98t31.fsf@redhat.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain Sean Christopherson writes: > Allocate, get, and free the CPUID array in the Hyper-V CPUID test in the > test's core helper, instead of copy+pasting code at each call site. In > addition to deduplicating a small amount of code, restricting visibility > of the array to a single invocation of the core test prevents "leaking" an > array across test cases. Passing in @vcpu to the helper will also allow > pivoting on VM-scoped information without needing to pass more booleans, > e.g. to conditionally assert on features that require an in-kernel APIC. > > To avoid use-after-free bugs due to overzealous and careless developers, > opportunstically add a comment to explain that the system-scoped helper > caches the Hyper-V CPUID entries, i.e. that the caller is not responsible > for freeing the memory. > > Cc: Vitaly Kuznetsov > Signed-off-by: Sean Christopherson > --- > .../selftests/kvm/x86_64/hyperv_cpuid.c | 30 +++++++++++-------- > 1 file changed, 17 insertions(+), 13 deletions(-) > > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c > index 9a0fcc713350..3188749ec6e1 100644 > --- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c > @@ -41,13 +41,18 @@ static bool smt_possible(void) > return res; > } > > -static void test_hv_cpuid(const struct kvm_cpuid2 *hv_cpuid_entries, > - bool evmcs_expected) > +static void test_hv_cpuid(struct kvm_vcpu *vcpu, bool evmcs_expected) > { > + const struct kvm_cpuid2 *hv_cpuid_entries; > int i; > int nent_expected = 10; > u32 test_val; > > + if (vcpu) > + hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu); > + else > + hv_cpuid_entries = kvm_get_supported_hv_cpuid(); > + > TEST_ASSERT(hv_cpuid_entries->nent == nent_expected, > "KVM_GET_SUPPORTED_HV_CPUID should return %d entries" > " (returned %d)", > @@ -109,6 +114,13 @@ static void test_hv_cpuid(const struct kvm_cpuid2 *hv_cpuid_entries, > * entry->edx); > */ > } > + > + /* > + * Note, the CPUID array returned by the system-scoped helper is a one- > + * time allocation, i.e. must not be freed. > + */ > + if (vcpu) > + free((void *)hv_cpuid_entries); > } > > static void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu) > @@ -129,7 +141,6 @@ static void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu) > int main(int argc, char *argv[]) > { > struct kvm_vm *vm; > - const struct kvm_cpuid2 *hv_cpuid_entries; > struct kvm_vcpu *vcpu; > > TEST_REQUIRE(kvm_has_cap(KVM_CAP_HYPERV_CPUID)); > @@ -138,10 +149,7 @@ int main(int argc, char *argv[]) > > /* Test vCPU ioctl version */ > test_hv_cpuid_e2big(vm, vcpu); > - > - hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu); > - test_hv_cpuid(hv_cpuid_entries, false); > - free((void *)hv_cpuid_entries); > + test_hv_cpuid(vcpu, false); > > if (!kvm_cpu_has(X86_FEATURE_VMX) || > !kvm_has_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) { > @@ -149,9 +157,7 @@ int main(int argc, char *argv[]) > goto do_sys; > } > vcpu_enable_evmcs(vcpu); > - hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu); > - test_hv_cpuid(hv_cpuid_entries, true); > - free((void *)hv_cpuid_entries); > + test_hv_cpuid(vcpu, true); > > do_sys: > /* Test system ioctl version */ > @@ -161,9 +167,7 @@ int main(int argc, char *argv[]) > } > > test_hv_cpuid_e2big(vm, NULL); > - > - hv_cpuid_entries = kvm_get_supported_hv_cpuid(); > - test_hv_cpuid(hv_cpuid_entries, kvm_cpu_has(X86_FEATURE_VMX)); > + test_hv_cpuid(NULL, kvm_cpu_has(X86_FEATURE_VMX)); > > out: > kvm_vm_free(vm); Reviewed-by: Vitaly Kuznetsov -- Vitaly