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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6E027C4167D for ; Tue, 7 Nov 2023 05:47:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 52B478D0034; Tue, 7 Nov 2023 00:47:56 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4D9D98D0001; Tue, 7 Nov 2023 00:47:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 37AC38D0034; Tue, 7 Nov 2023 00:47:56 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 28A368D0001 for ; Tue, 7 Nov 2023 00:47:56 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id DFBAA12070C for ; Tue, 7 Nov 2023 05:47:55 +0000 (UTC) X-FDA: 81430076910.08.9EB815E Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.7]) by imf03.hostedemail.com (Postfix) with ESMTP id 553EA20006 for ; Tue, 7 Nov 2023 05:47:53 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=NGFaIJ0b; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf03.hostedemail.com: domain of yuan.yao@linux.intel.com has no SPF policy when checking 192.198.163.7) smtp.mailfrom=yuan.yao@linux.intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1699336073; a=rsa-sha256; cv=none; b=XHp0YGvJI7fJdAzlP+Fl8ZKiDa+E04D3xaznBKOMdv2P5KhBKRsEl8GlWVe1COH4/o/XL+ bXrGmZeQ1McFMVktnSGYjEWBuNB+w71WKNNRBdLzHurau13q/JKbvwBcm9ALQ9ODXDn14m 4pFUbLcybatVZ68ETvw198SVks0dEZY= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=NGFaIJ0b; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf03.hostedemail.com: domain of yuan.yao@linux.intel.com has no SPF policy when checking 192.198.163.7) smtp.mailfrom=yuan.yao@linux.intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1699336073; h=from:from:sender: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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=B+mCMdC5Tj2mkXgj+BaY8ic4+RLvoy2mCKHEGC2lGyg=; b=AlSgDWlBlljZBs6sCs1VJCqiEr0W6D3HRbn6EjwzFTGXCytKlUdyNcRSNjMjWhQMdPSSyJ y90XmAfX1WLE4seeDiUgtLntLXbny7mcUc9SSn06TcHVhFvMYiP2eKCWvmoxflSyv2PVm7 vnvQKGd2JwveFUywMJ2B0oJXZjEnMWg= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1699336074; x=1730872074; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=oqZ5JQNUaGOSPFfGCcRkwAJc+NX/2BP9g4PZc+AIHn4=; b=NGFaIJ0b6TFt5BWPtxtbslSPizzpxp9Qgi0lroZIlA++B+WpeNcABHWk WVLMGiAN9KMWHWR9VVvqCAnJvxsHkZZAn3aEI6pOnMzWwHm2GM0p1P4gC OjDqkzEeI0k71B4MBe44BTuVRwET0jgtt19cMSi6K/xpqAnqlR8oYZ5+6 dKhSnVmagoY1L/3sTs9ZCiZI/75/k/UyA4aDnexZPWxPVDNzWUHVza0pl sRvHTWOCp67L6ATrHnfjw394flGZaRM13TsLf2jlSOPhslp8q8lDTOW7P CtvNx5auTXCvN/nQOWBnAG1Q/IuvMG8hmoLw5IILg1DgJF2XVgtRgxG4z g==; X-IronPort-AV: E=McAfee;i="6600,9927,10886"; a="10978176" X-IronPort-AV: E=Sophos;i="6.03,282,1694761200"; d="scan'208";a="10978176" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmvoesa101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Nov 2023 21:47:51 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10886"; a="832964536" X-IronPort-AV: E=Sophos;i="6.03,282,1694761200"; d="scan'208";a="832964536" Received: from yy-desk-7060.sh.intel.com (HELO localhost) ([10.239.159.76]) by fmsmga004.fm.intel.com with ESMTP; 06 Nov 2023 21:47:40 -0800 Date: Tue, 7 Nov 2023 13:47:40 +0800 From: Yuan Yao To: Paolo Bonzini Cc: Marc Zyngier , Oliver Upton , Huacai Chen , Michael Ellerman , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Sean Christopherson , Alexander Viro , Christian Brauner , "Matthew Wilcox (Oracle)" , Andrew Morton , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Xiaoyao Li , Xu Yilun , Chao Peng , Fuad Tabba , Jarkko Sakkinen , Anish Moorthy , David Matlack , Yu Zhang , Isaku Yamahata , =?utf-8?Q?Micka=C3=ABl_Sala=C3=BCn?= , Vlastimil Babka , Vishal Annapurve , Ackerley Tng , Maciej Szmigiero , David Hildenbrand , Quentin Perret , Michael Roth , Wang , Liam Merwick , Isaku Yamahata , "Kirill A. Shutemov" Subject: Re: [PATCH 08/34] KVM: Introduce KVM_SET_USER_MEMORY_REGION2 Message-ID: <20231107054739.pamcdd2z564c6xv3@yy-desk-7060> References: <20231105163040.14904-1-pbonzini@redhat.com> <20231105163040.14904-9-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231105163040.14904-9-pbonzini@redhat.com> User-Agent: NeoMutt/20171215 X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 553EA20006 X-Stat-Signature: 6czo9xydegc3zmrt9i8npqu5pniajkdj X-HE-Tag: 1699336073-532809 X-HE-Meta: U2FsdGVkX1+6nBXocNS1r43RL6AF+fLLfttGu8KAjGvTT7Y74hYG3tmWi2dAk4Acosv/8KT811HxE04mZdlDJbSdkD3vsrq3aRBpzKN5FhjSCENXbL8heTOvaTudlBGYhj3Fz0vWCikIIJFK5amEx8QYou8XGoGFXLHouYbCcAPHkMWqXI8Td3x1djTE0LUyPF0J2nKr9MExZ64rLUE9UgBCmRm90C37eJBF04u4v9GjJLU0hzUOLh899N4Zgxj7+PemAaN59bARWUyvLYgsFjdDxObnckJPyct9RUXnmAR9ank6twuRdQf4u6qHEk8k6C7vk6kBZ40LKgJOQ+KL/349+HvGBxYO159h82d6xYCj/hi5LcP8id1c4OH86SIezKp11i9hBr2XBr34lVD9pPInK014H1BnPkDRPYf8H907E0goGwLmKXF6dkJDr5BtDaRYyd0atCIv8c5gPJm32YiPdTQ0nRMgN0e8ioDYOEaBQs5ke3cwjfsSMKgE+0S5PZFJRRGtmZRWgFyQirt5sSOfbOnN7f1yd87MP9ZoEAGtVswRjK+E14SJTtonEFUYjKBgD70vzaOq+Z9yhVlBBMvZzBD7GGe4FK9ZX901hmLDxYjMm6npwBOltU5nWzooTC08VrMnRGaEp5NERCLQWb7ENMN9No6X1GVcICOz4HHWWbhe2xm64Uol5PY1RXXlaBCwpo41mxhMh7kq4AfAkEFS3Ce5KQx0fSRx8w9TwOLx9ZjaPBwJ9nI8m1GNb/wdAEPSzgFkwJQEC21P+kQtVddVbcAl2f1kHhp2JvpoU1GOdIVUqnusqboII1YDXE7NNo+fmiTGnETghQGg8Q2Ioh8d8cIeee1ItZQ6sX9eYm44al1s4RxKHREoEcKTZU1pytacrke49tTr6LwxkuGC+vERaoxZiv8PajyjJyifayAsobcxYKMAm4ViU59r2C/bn9VFUqlH7ariW86fIbx LUdltwuD tVgO4xG4X62P2JUO0yxQPixGfAEGVo/OOzlXVakHl3D7XYPfAgKroAfvhVtQwMWuvnOSRa2+7yxQMPrEaKwFGxpCqFey3Lxerygh3LGALxCn4gZGGN+AEIPKsSAJmQdKzWKUMWLW4pettj/GQU8A/SIICYgTs9ZRvXLWmyBcVpWpJxnx8q5QsO4bCgP1JpwSaDrFF5THUwkDu3DEKZi9mGsiRs68PVksfY5FURbFb2NzwSNb59UiS6Sf08U8ctnoaZWftTDfIP4iwuTAwkt5KZ9RkkcGNDQbrW30JoiSM/UwCrhuCWLW7p3YN1HrURWBjMGi4uxpcNeuTQc2hB3W5Hc+XWmfDiqcmSBDDAN1kfejlW5u6vyg8cpP7NA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Sun, Nov 05, 2023 at 05:30:11PM +0100, Paolo Bonzini wrote: > From: Sean Christopherson > > Introduce a "version 2" of KVM_SET_USER_MEMORY_REGION so that additional > information can be supplied without setting userspace up to fail. The > padding in the new kvm_userspace_memory_region2 structure will be used to > pass a file descriptor in addition to the userspace_addr, i.e. allow > userspace to point at a file descriptor and map memory into a guest that > is NOT mapped into host userspace. > > Alternatively, KVM could simply add "struct kvm_userspace_memory_region2" > without a new ioctl(), but as Paolo pointed out, adding a new ioctl() > makes detection of bad flags a bit more robust, e.g. if the new fd field > is guarded only by a flag and not a new ioctl(), then a userspace bug > (setting a "bad" flag) would generate out-of-bounds access instead of an > -EINVAL error. > > Cc: Jarkko Sakkinen > Reviewed-by: Paolo Bonzini > Reviewed-by: Xiaoyao Li > Signed-off-by: Sean Christopherson > Reviewed-by: Fuad Tabba > Tested-by: Fuad Tabba > Message-Id: <20231027182217.3615211-9-seanjc@google.com> > Signed-off-by: Paolo Bonzini > --- > Documentation/virt/kvm/api.rst | 22 +++++++++++++ > arch/x86/kvm/x86.c | 2 +- > include/linux/kvm_host.h | 4 +-- > include/uapi/linux/kvm.h | 13 ++++++++ > virt/kvm/kvm_main.c | 57 +++++++++++++++++++++++++++++----- > 5 files changed, 87 insertions(+), 11 deletions(-) > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index 7025b3751027..bdea1423c5f8 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -1340,6 +1340,7 @@ yet and must be cleared on entry. > __u64 guest_phys_addr; > __u64 memory_size; /* bytes */ > __u64 userspace_addr; /* start of the userspace allocated memory */ > + __u64 pad[16]; Looks incorrect to add padding part in kvm_userspace_memory_region, only need to apply on kvm_userspace_memory_region2 below. > }; > > /* for kvm_userspace_memory_region::flags */ > @@ -6192,6 +6193,27 @@ to know what fields can be changed for the system register described by > ``op0, op1, crn, crm, op2``. KVM rejects ID register values that describe a > superset of the features supported by the system. > > +4.140 KVM_SET_USER_MEMORY_REGION2 > +--------------------------------- > + > +:Capability: KVM_CAP_USER_MEMORY2 > +:Architectures: all > +:Type: vm ioctl > +:Parameters: struct kvm_userspace_memory_region2 (in) > +:Returns: 0 on success, -1 on error > + > +:: > + > + struct kvm_userspace_memory_region2 { > + __u32 slot; > + __u32 flags; > + __u64 guest_phys_addr; > + __u64 memory_size; /* bytes */ > + __u64 userspace_addr; /* start of the userspace allocated memory */ > + }; > + > +See KVM_SET_USER_MEMORY_REGION. > + > 5. The kvm_run structure > ======================== > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 2c924075f6f1..7b389f27dffc 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -12576,7 +12576,7 @@ void __user * __x86_set_memory_region(struct kvm *kvm, int id, gpa_t gpa, > } > > for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++) { > - struct kvm_userspace_memory_region m; > + struct kvm_userspace_memory_region2 m; > > m.slot = id | (i << 16); > m.flags = 0; > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 5faba69403ac..4e741ff27af3 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -1146,9 +1146,9 @@ enum kvm_mr_change { > }; > > int kvm_set_memory_region(struct kvm *kvm, > - const struct kvm_userspace_memory_region *mem); > + const struct kvm_userspace_memory_region2 *mem); > int __kvm_set_memory_region(struct kvm *kvm, > - const struct kvm_userspace_memory_region *mem); > + const struct kvm_userspace_memory_region2 *mem); > void kvm_arch_free_memslot(struct kvm *kvm, struct kvm_memory_slot *slot); > void kvm_arch_memslots_updated(struct kvm *kvm, u64 gen); > int kvm_arch_prepare_memory_region(struct kvm *kvm, > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index 211b86de35ac..308cc70bd6ab 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -95,6 +95,16 @@ struct kvm_userspace_memory_region { > __u64 userspace_addr; /* start of the userspace allocated memory */ > }; > > +/* for KVM_SET_USER_MEMORY_REGION2 */ > +struct kvm_userspace_memory_region2 { > + __u32 slot; > + __u32 flags; > + __u64 guest_phys_addr; > + __u64 memory_size; > + __u64 userspace_addr; > + __u64 pad[16]; > +}; > + > /* > * The bit 0 ~ bit 15 of kvm_userspace_memory_region::flags are visible for > * userspace, other bits are reserved for kvm internal use which are defined > @@ -1201,6 +1211,7 @@ struct kvm_ppc_resize_hpt { > #define KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE 228 > #define KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES 229 > #define KVM_CAP_ARM_SUPPORTED_REG_MASK_RANGES 230 > +#define KVM_CAP_USER_MEMORY2 231 > > #ifdef KVM_CAP_IRQ_ROUTING > > @@ -1483,6 +1494,8 @@ struct kvm_vfio_spapr_tce { > struct kvm_userspace_memory_region) > #define KVM_SET_TSS_ADDR _IO(KVMIO, 0x47) > #define KVM_SET_IDENTITY_MAP_ADDR _IOW(KVMIO, 0x48, __u64) > +#define KVM_SET_USER_MEMORY_REGION2 _IOW(KVMIO, 0x49, \ > + struct kvm_userspace_memory_region2) > > /* enable ucontrol for s390 */ > struct kvm_s390_ucas_mapping { > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index dc81279ea385..756b94ecd511 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -1580,7 +1580,15 @@ static void kvm_replace_memslot(struct kvm *kvm, > } > } > > -static int check_memory_region_flags(const struct kvm_userspace_memory_region *mem) > +/* > + * Flags that do not access any of the extra space of struct > + * kvm_userspace_memory_region2. KVM_SET_USER_MEMORY_REGION_V1_FLAGS > + * only allows these. > + */ > +#define KVM_SET_USER_MEMORY_REGION_V1_FLAGS \ > + (KVM_MEM_LOG_DIRTY_PAGES | KVM_MEM_READONLY) > + > +static int check_memory_region_flags(const struct kvm_userspace_memory_region2 *mem) > { > u32 valid_flags = KVM_MEM_LOG_DIRTY_PAGES; > > @@ -1982,7 +1990,7 @@ static bool kvm_check_memslot_overlap(struct kvm_memslots *slots, int id, > * Must be called holding kvm->slots_lock for write. > */ > int __kvm_set_memory_region(struct kvm *kvm, > - const struct kvm_userspace_memory_region *mem) > + const struct kvm_userspace_memory_region2 *mem) > { > struct kvm_memory_slot *old, *new; > struct kvm_memslots *slots; > @@ -2086,7 +2094,7 @@ int __kvm_set_memory_region(struct kvm *kvm, > EXPORT_SYMBOL_GPL(__kvm_set_memory_region); > > int kvm_set_memory_region(struct kvm *kvm, > - const struct kvm_userspace_memory_region *mem) > + const struct kvm_userspace_memory_region2 *mem) > { > int r; > > @@ -2098,7 +2106,7 @@ int kvm_set_memory_region(struct kvm *kvm, > EXPORT_SYMBOL_GPL(kvm_set_memory_region); > > static int kvm_vm_ioctl_set_memory_region(struct kvm *kvm, > - struct kvm_userspace_memory_region *mem) > + struct kvm_userspace_memory_region2 *mem) > { > if ((u16)mem->slot >= KVM_USER_MEM_SLOTS) > return -EINVAL; > @@ -4568,6 +4576,7 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg) > { > switch (arg) { > case KVM_CAP_USER_MEMORY: > + case KVM_CAP_USER_MEMORY2: > case KVM_CAP_DESTROY_MEMORY_REGION_WORKS: > case KVM_CAP_JOIN_MEMORY_REGIONS_WORKS: > case KVM_CAP_INTERNAL_ERROR_DATA: > @@ -4823,6 +4832,14 @@ static int kvm_vm_ioctl_get_stats_fd(struct kvm *kvm) > return fd; > } > > +#define SANITY_CHECK_MEM_REGION_FIELD(field) \ > +do { \ > + BUILD_BUG_ON(offsetof(struct kvm_userspace_memory_region, field) != \ > + offsetof(struct kvm_userspace_memory_region2, field)); \ > + BUILD_BUG_ON(sizeof_field(struct kvm_userspace_memory_region, field) != \ > + sizeof_field(struct kvm_userspace_memory_region2, field)); \ > +} while (0) > + > static long kvm_vm_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg) > { > @@ -4845,15 +4862,39 @@ static long kvm_vm_ioctl(struct file *filp, > r = kvm_vm_ioctl_enable_cap_generic(kvm, &cap); > break; > } > + case KVM_SET_USER_MEMORY_REGION2: > case KVM_SET_USER_MEMORY_REGION: { > - struct kvm_userspace_memory_region kvm_userspace_mem; > + struct kvm_userspace_memory_region2 mem; > + unsigned long size; > + > + if (ioctl == KVM_SET_USER_MEMORY_REGION) { > + /* > + * Fields beyond struct kvm_userspace_memory_region shouldn't be > + * accessed, but avoid leaking kernel memory in case of a bug. > + */ > + memset(&mem, 0, sizeof(mem)); > + size = sizeof(struct kvm_userspace_memory_region); > + } else { > + size = sizeof(struct kvm_userspace_memory_region2); > + } > + > + /* Ensure the common parts of the two structs are identical. */ > + SANITY_CHECK_MEM_REGION_FIELD(slot); > + SANITY_CHECK_MEM_REGION_FIELD(flags); > + SANITY_CHECK_MEM_REGION_FIELD(guest_phys_addr); > + SANITY_CHECK_MEM_REGION_FIELD(memory_size); > + SANITY_CHECK_MEM_REGION_FIELD(userspace_addr); > > r = -EFAULT; > - if (copy_from_user(&kvm_userspace_mem, argp, > - sizeof(kvm_userspace_mem))) > + if (copy_from_user(&mem, argp, size)) > goto out; > > - r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem); > + r = -EINVAL; > + if (ioctl == KVM_SET_USER_MEMORY_REGION && > + (mem.flags & ~KVM_SET_USER_MEMORY_REGION_V1_FLAGS)) > + goto out; > + > + r = kvm_vm_ioctl_set_memory_region(kvm, &mem); > break; > } > case KVM_GET_DIRTY_LOG: { > -- > 2.39.1 > > >