From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Christopherson Date: Fri, 3 Feb 2023 17:48:49 +0000 Subject: [PATCH 2/7] KVM: x86: Improve return type handling in kvm_vm_ioctl_get_nr_mmu_pages() In-Reply-To: <20230203094230.266952-3-thuth@redhat.com> References: <20230203094230.266952-1-thuth@redhat.com> <20230203094230.266952-3-thuth@redhat.com> Message-ID: List-Id: To: kvm-riscv@lists.infradead.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Fri, Feb 03, 2023, Thomas Huth wrote: > kvm_vm_ioctl_get_nr_mmu_pages() tries to return a "unsigned long" value, > but its caller only stores ther return value in an "int" - which is also > what all the other kvm_vm_ioctl_*() functions are returning. So returning > values that do not fit into a 32-bit integer anymore does not work here. > It's better to adjust the return type, add a sanity check and return an > error instead if the value is too big. > > Signed-off-by: Thomas Huth > --- > arch/x86/kvm/x86.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index da4bbd043a7b..caa2541833dd 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6007,8 +6007,11 @@ static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm, > return 0; > } > > -static unsigned long kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm) > +static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm) > { > + if (kvm->arch.n_max_mmu_pages > INT_MAX) > + return -EOVERFLOW; > + > return kvm->arch.n_max_mmu_pages; > } My vote is to skip this patch, skip deprecation, and go straight to deleting KVM_GET_NR_MMU_PAGES. The ioctl() has never worked[*], and none of the VMMs I checked use it (QEMU, Google's internal VMM, kvmtool, CrosVM). [*] https://lore.kernel.org/all/YpZu6%2Fk+8EydfBKf at google.com From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f42.google.com (mail-pj1-f42.google.com [209.85.216.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 37A79AD3A for ; Fri, 3 Feb 2023 17:48:55 +0000 (UTC) Received: by mail-pj1-f42.google.com with SMTP id o13so5759642pjg.2 for ; Fri, 03 Feb 2023 09:48:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=1t48P/jVd6+3dhw3pck4JM1odaqwThYt8bjKIe9FySY=; b=YUGCo4ZbvxduWRBHH9xOGxUv8LoaXyN+VOL8K8Kve/u16uDaalEXH+kBO1C3PAOZdM U2FdeDmTEYvv/Khm9Hp1pGOcZfLi1gl8qmki4SmGN3WfKIsCemaDy6HsGXS8a2DwuvSq p7nNVpP/rnqBe+1jiPBY7eVjGaGnTjSVk5H8VHlpO4DlQHD7k1TC35BcdqTcy2ZQRaSF 4md/KQ7IeYFOS2FYZ+qMRDUx2NMJplrAtBdVvGxGX0DbpsM7eAfg2iIiZtKPhF8ngclz nFNAH/NrZwg3lKNjgzMI4QsgFt81DdqqGb/JrZPeZ3VqGuwxbqlyijT98K4d4Jb58ATa yKNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=1t48P/jVd6+3dhw3pck4JM1odaqwThYt8bjKIe9FySY=; b=ABbK1KlZhSFPoMg3HN7bjcAqG4Vyfp2a9XYeK6qHJQhax4PfZca8cimJRy2JGLP8CV IkngKP54Wv34baHnGDFciaKVjxfvuQ7wsWDi7weowpyKcRfBNYQFvOcF7XGY57yRo+Rl ViTykI/6HaFSI/T8gi/MTOQRr1W2qccoyLy7fRBmSBE8d44HQr6vyBxa7VNd6lam8Uo/ dc8vkpEkVas2pgaHQEf83MVhTS0gq15EtJvGIJFW/9Ap78c2a8SJZDcpKZLJmb9DL+fu x1cZOqQ5vgU78J/qGPPjcoUv/AGYQS/OsnMLaS4Jheh1g3W+SABXAM/OrHNMBUgIpPPW EZzg== X-Gm-Message-State: AO0yUKWQvwvh5VAvQhl8YS0iIBzPl87vEsfg+h5+9TljvDsdgsQhWiZm nsicNzQqYxaTHoC3br+brD+i0g== X-Google-Smtp-Source: AK7set8NtYpsGJ6byRewhpDzAYHCL4kVnwYOuRswQz6j6iO+lQNMoz+KA+gf7NkDr7m6tVMm7U3Fmg== X-Received: by 2002:a17:902:d1d5:b0:198:d5cc:44a8 with SMTP id g21-20020a170902d1d500b00198d5cc44a8mr260726plb.19.1675446534414; Fri, 03 Feb 2023 09:48:54 -0800 (PST) Received: from google.com (7.104.168.34.bc.googleusercontent.com. [34.168.104.7]) by smtp.gmail.com with ESMTPSA id b2-20020a1709027e0200b00194b3a7853esm1865998plm.181.2023.02.03.09.48.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 03 Feb 2023 09:48:53 -0800 (PST) Date: Fri, 3 Feb 2023 17:48:49 +0000 From: Sean Christopherson To: Thomas Huth Cc: kvm@vger.kernel.org, Paolo Bonzini , kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, kvm-riscv@lists.infradead.org, Marc Zyngier , James Morse , Suzuki K Poulose , Oliver Upton , Zenghui Yu , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , David Hildenbrand , linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 2/7] KVM: x86: Improve return type handling in kvm_vm_ioctl_get_nr_mmu_pages() Message-ID: References: <20230203094230.266952-1-thuth@redhat.com> <20230203094230.266952-3-thuth@redhat.com> Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230203094230.266952-3-thuth@redhat.com> On Fri, Feb 03, 2023, Thomas Huth wrote: > kvm_vm_ioctl_get_nr_mmu_pages() tries to return a "unsigned long" value, > but its caller only stores ther return value in an "int" - which is also > what all the other kvm_vm_ioctl_*() functions are returning. So returning > values that do not fit into a 32-bit integer anymore does not work here. > It's better to adjust the return type, add a sanity check and return an > error instead if the value is too big. > > Signed-off-by: Thomas Huth > --- > arch/x86/kvm/x86.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index da4bbd043a7b..caa2541833dd 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6007,8 +6007,11 @@ static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm, > return 0; > } > > -static unsigned long kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm) > +static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm) > { > + if (kvm->arch.n_max_mmu_pages > INT_MAX) > + return -EOVERFLOW; > + > return kvm->arch.n_max_mmu_pages; > } My vote is to skip this patch, skip deprecation, and go straight to deleting KVM_GET_NR_MMU_PAGES. The ioctl() has never worked[*], and none of the VMMs I checked use it (QEMU, Google's internal VMM, kvmtool, CrosVM). [*] https://lore.kernel.org/all/YpZu6%2Fk+8EydfBKf@google.com 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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DCF58C05027 for ; Fri, 3 Feb 2023 17:49:54 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4P7jpm6bgCz3f8n for ; Sat, 4 Feb 2023 04:49:52 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20210112 header.b=YUGCo4Zb; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=google.com (client-ip=2607:f8b0:4864:20::102c; helo=mail-pj1-x102c.google.com; envelope-from=seanjc@google.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20210112 header.b=YUGCo4Zb; dkim-atps=neutral Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4P7jnl0112z2ynf for ; Sat, 4 Feb 2023 04:48:57 +1100 (AEDT) Received: by mail-pj1-x102c.google.com with SMTP id f16-20020a17090a9b1000b0023058bbd7b2so5020185pjp.0 for ; Fri, 03 Feb 2023 09:48:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=1t48P/jVd6+3dhw3pck4JM1odaqwThYt8bjKIe9FySY=; b=YUGCo4ZbvxduWRBHH9xOGxUv8LoaXyN+VOL8K8Kve/u16uDaalEXH+kBO1C3PAOZdM U2FdeDmTEYvv/Khm9Hp1pGOcZfLi1gl8qmki4SmGN3WfKIsCemaDy6HsGXS8a2DwuvSq p7nNVpP/rnqBe+1jiPBY7eVjGaGnTjSVk5H8VHlpO4DlQHD7k1TC35BcdqTcy2ZQRaSF 4md/KQ7IeYFOS2FYZ+qMRDUx2NMJplrAtBdVvGxGX0DbpsM7eAfg2iIiZtKPhF8ngclz nFNAH/NrZwg3lKNjgzMI4QsgFt81DdqqGb/JrZPeZ3VqGuwxbqlyijT98K4d4Jb58ATa yKNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=1t48P/jVd6+3dhw3pck4JM1odaqwThYt8bjKIe9FySY=; b=VRkuiKY5Lf6QyJp8HzNxNTvQ2VafnSZuQMIqoY3wZVQ5I7wTbY0a7jVkMOrdLtVhLL JAEGKkWySoQs9OrD6v5oku8EalEQpWsndY1bfv13du3+D9xGcyZ6rndK314/fgvL1fi1 8JMocCZFytRIg8FVZRQWl6xT9MRZN21isdy45CZc4LUUwaw5uFwQ19+vRhxijX28JpVU xBR7j+JpWxBoEENAzP3pWQ5DSPaM6bUPXumOHJIIlUqUKmG4PT5DPbtbhqqjLDWwK3mz wOrHoPLcmbkDSVN482pJI5MDJofNP7rDwfiBT31jZwGuG5iFuY38oiTm+zoTlcPn0hyd FQ3w== X-Gm-Message-State: AO0yUKVfX+6A3ISH6EVRmnJ3Fmjevx0XabkChcXtVriEq023CnUxrHJf 6SWIEYZbCXOV+Y2ps0ISQ4C40Q== X-Google-Smtp-Source: AK7set8NtYpsGJ6byRewhpDzAYHCL4kVnwYOuRswQz6j6iO+lQNMoz+KA+gf7NkDr7m6tVMm7U3Fmg== X-Received: by 2002:a17:902:d1d5:b0:198:d5cc:44a8 with SMTP id g21-20020a170902d1d500b00198d5cc44a8mr260726plb.19.1675446534414; Fri, 03 Feb 2023 09:48:54 -0800 (PST) Received: from google.com (7.104.168.34.bc.googleusercontent.com. [34.168.104.7]) by smtp.gmail.com with ESMTPSA id b2-20020a1709027e0200b00194b3a7853esm1865998plm.181.2023.02.03.09.48.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 03 Feb 2023 09:48:53 -0800 (PST) Date: Fri, 3 Feb 2023 17:48:49 +0000 From: Sean Christopherson To: Thomas Huth Subject: Re: [PATCH 2/7] KVM: x86: Improve return type handling in kvm_vm_ioctl_get_nr_mmu_pages() Message-ID: References: <20230203094230.266952-1-thuth@redhat.com> <20230203094230.266952-3-thuth@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230203094230.266952-3-thuth@redhat.com> X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Claudio Imbrenda , Janosch Frank , kvm@vger.kernel.org, Suzuki K Poulose , Marc Zyngier , David Hildenbrand , linux-kernel@vger.kernel.org, Oliver Upton , Zenghui Yu , James Morse , kvm-riscv@lists.infradead.org, kvmarm@lists.linux.dev, Paolo Bonzini , Christian Borntraeger , linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Fri, Feb 03, 2023, Thomas Huth wrote: > kvm_vm_ioctl_get_nr_mmu_pages() tries to return a "unsigned long" value, > but its caller only stores ther return value in an "int" - which is also > what all the other kvm_vm_ioctl_*() functions are returning. So returning > values that do not fit into a 32-bit integer anymore does not work here. > It's better to adjust the return type, add a sanity check and return an > error instead if the value is too big. > > Signed-off-by: Thomas Huth > --- > arch/x86/kvm/x86.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index da4bbd043a7b..caa2541833dd 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6007,8 +6007,11 @@ static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm, > return 0; > } > > -static unsigned long kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm) > +static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm) > { > + if (kvm->arch.n_max_mmu_pages > INT_MAX) > + return -EOVERFLOW; > + > return kvm->arch.n_max_mmu_pages; > } My vote is to skip this patch, skip deprecation, and go straight to deleting KVM_GET_NR_MMU_PAGES. The ioctl() has never worked[*], and none of the VMMs I checked use it (QEMU, Google's internal VMM, kvmtool, CrosVM). [*] https://lore.kernel.org/all/YpZu6%2Fk+8EydfBKf@google.com