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 X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D5CCFC35673 for ; Mon, 24 Feb 2020 01:44:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B429420675 for ; Mon, 24 Feb 2020 01:44:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727236AbgBXBor convert rfc822-to-8bit (ORCPT ); Sun, 23 Feb 2020 20:44:47 -0500 Received: from szxga08-in.huawei.com ([45.249.212.255]:48706 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727151AbgBXBor (ORCPT ); Sun, 23 Feb 2020 20:44:47 -0500 Received: from DGGEMM401-HUB.china.huawei.com (unknown [172.30.72.53]) by Forcepoint Email with ESMTP id 3F9634FCA9B9E5C9F61D; Mon, 24 Feb 2020 09:44:44 +0800 (CST) Received: from dggeme752-chm.china.huawei.com (10.3.19.98) by DGGEMM401-HUB.china.huawei.com (10.3.20.209) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 24 Feb 2020 09:44:43 +0800 Received: from dggeme753-chm.china.huawei.com (10.3.19.99) by dggeme752-chm.china.huawei.com (10.3.19.98) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1713.5; Mon, 24 Feb 2020 09:44:43 +0800 Received: from dggeme753-chm.china.huawei.com ([10.7.64.70]) by dggeme753-chm.china.huawei.com ([10.7.64.70]) with mapi id 15.01.1713.004; Mon, 24 Feb 2020 09:44:43 +0800 From: linmiaohe To: Sean Christopherson CC: "pbonzini@redhat.com" , "rkrcmar@redhat.com" , "vkuznets@redhat.com" , "wanpengli@tencent.com" , "jmattson@google.com" , "joro@8bytes.org" , "tglx@linutronix.de" , "mingo@redhat.com" , "bp@alien8.de" , "hpa@zytor.com" , "kvm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "x86@kernel.org" Subject: Re: [PATCH] KVM: X86: eliminate some meaningless code Thread-Topic: [PATCH] KVM: X86: eliminate some meaningless code Thread-Index: AdXqsyQk+rut2bDwRPO7k/XEvhn+sQ== Date: Mon, 24 Feb 2020 01:44:43 +0000 Message-ID: <5ac7a51dbcc7408d87b14be75b41f1dc@huawei.com> Accept-Language: en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.173.221.158] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Sean Christopherson wrote: > On Fri, Feb 21, 2020 at 10:05:26PM +0800, linmiaohe wrote: >> From: Miaohe Lin >> >> When kvm_vcpu_ioctl_get_cpuid2() fails, we set cpuid->nent to the >> value of >> vcpu->arch.cpuid_nent. But this is in vain as cpuid->nent is not >> vcpu->copied to >> userspace by copy_to_user() from call site. Get rid of this >> meaningless assignment and further cleanup the var r and out jump label. > >Ha, took me a while to see that. Sorry about it. I'am not good at it. :( >> >> On the other hand, when kvm_vcpu_ioctl_get_cpuid2() succeeds, we do >> vcpu->arch.cpuid_nent * sizeof(struct kvm_cpuid_entry2))) >> - goto out; >> - return 0; > >Hmm, so this ioctl() is straight up broken. cpuid->nent should be updated on success so that userspace knows how many entries were retrieved, i.e. >the code should look something like below, with kvm_arch_vcpu_ioctl() unchanged. > >I'm guessing no VMM actually uses this ioctl(), e.g. neither Qemu or CrosVM use it, which is why the broken behavior has gone unnoticed. Don't suppose you'd want to write a selftest to hammer KVM_{SET,GET}_CPUID2? > >int kvm_vcpu_ioctl_get_cpuid2(struct kvm_vcpu *vcpu, > struct kvm_cpuid2 *cpuid, > struct kvm_cpuid_entry2 __user *entries) { > if (cpuid->nent < vcpu->arch.cpuid_nent) > return -E2BIG; > > if (copy_to_user(entries, &vcpu->arch.cpuid_entries, > vcpu->arch.cpuid_nent * sizeof(struct kvm_cpuid_entry2))) > return -EFAULT; > > cpuid->nent = vcpu->arch.cpuid_nent; > > return 0; >} > I searched KVM_GET_CPUID2 from Qemu, it's not used. So maybe we could just drop KVM_GET_CPUID2 altogether as suggested by Paolo. Thanks for your review.