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 14FC8C433FE for ; Mon, 28 Mar 2022 22:27:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 61A6C8D0002; Mon, 28 Mar 2022 18:27:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5C9508D0001; Mon, 28 Mar 2022 18:27:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4B8268D0002; Mon, 28 Mar 2022 18:27:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0072.hostedemail.com [216.40.44.72]) by kanga.kvack.org (Postfix) with ESMTP id 3D8B48D0001 for ; Mon, 28 Mar 2022 18:27:02 -0400 (EDT) Received: from smtpin31.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id DADF61828A7F7 for ; Mon, 28 Mar 2022 22:27:01 +0000 (UTC) X-FDA: 79295231442.31.9CBB501 Received: from mail-pg1-f175.google.com (mail-pg1-f175.google.com [209.85.215.175]) by imf15.hostedemail.com (Postfix) with ESMTP id 739B4A003A for ; Mon, 28 Mar 2022 22:27:01 +0000 (UTC) Received: by mail-pg1-f175.google.com with SMTP id b130so11833996pga.13 for ; Mon, 28 Mar 2022 15:27:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=EaW6LnFnD0yHSZLOEoOdmGP4SdnTb3yLLH8IPqs77KY=; b=Xyvle5xj+iytheGd+opSaVUcJMrxydFolbbLLB0cfjcq5NLgRpKcyY8LmvKaLgqjGf 4Wz+LYcSQ0FaI+N5y+VY4SzfRHCjtAZByjFVMXJ2KOnIAwrohQC+hThWqUCVqG+4NbES 3SNF28Gr4dRvSXnzRTTpxIDvLztpdn1WZNrxh8Q8n7iGr0rCaj4r9+fp3XG393ZBWs6k SHSEkJqmZ3YIGgDpQAEgtvU4ck1H0tPnbq3AaPzkvTPpDKGc7uoyX0A9YsGMVeSengHe iGHaJiaF81xnBQn+5IhcSf8NxcBCjbS8eFYH7svcMpAomWcPYrKr0MsWl6ZnU5x0GLV1 TUTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=EaW6LnFnD0yHSZLOEoOdmGP4SdnTb3yLLH8IPqs77KY=; b=rUKz0TNh16ct7senSPdRe6Jv0a5Rfw1XVA1qmE6HJLg7FnAgCi+SOtYfkR7Hu51RlS hzlSs4VqmzFCJE52wcT4db9rtsyKpnLfg8VsmiG4g/g8QOhGAghOvCYoDGhsljKc9+YY fALT7jIgP318kBxDmhrOYQZiwuVYpihz4ZzLKmCUtvRRNOHrizAQFt97ointZbUZ9Yof R9HC0mVZlI+yTmyqaHS2kEmNO7DAibyeIUw3jysAOUPYryo0xB9sGgJ0bWlNDwOOsbR7 rUUTE9zkTccpSIwEdiVcK11aRBxwPOrsjYZAW1GD92t8Rcre8oWJU2VU5NbdiNYpundO Fk0Q== X-Gm-Message-State: AOAM531OaeQ3THkIjtbfKzXT261KJITstO1huGI/NadZ+A50vzUOQv+A TDUfXepgqgNTdycZMp+259jzrQ== X-Google-Smtp-Source: ABdhPJyCCih0kMQlx0T3Lnxi+OwtGBTbKYzIPbgZGi6J5hxjxW3HSWsT0UQDj72qn+FBmxrSiRdwKg== X-Received: by 2002:a62:e215:0:b0:4fa:87f1:dc16 with SMTP id a21-20020a62e215000000b004fa87f1dc16mr25067148pfi.19.1648506420043; Mon, 28 Mar 2022 15:27:00 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id f16-20020a056a001ad000b004fb358ffe86sm8955991pfv.137.2022.03.28.15.26.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Mar 2022 15:26:59 -0700 (PDT) Date: Mon, 28 Mar 2022 22:26:55 +0000 From: Sean Christopherson To: Chao Peng Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org, qemu-devel@nongnu.org, Paolo Bonzini , Jonathan Corbet , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , Thomas Gleixner , Ingo Molnar , Borislav Petkov , x86@kernel.org, "H . Peter Anvin" , Hugh Dickins , Jeff Layton , "J . Bruce Fields" , Andrew Morton , Mike Rapoport , Steven Price , "Maciej S . Szmigiero" , Vlastimil Babka , Vishal Annapurve , Yu Zhang , "Kirill A . Shutemov" , luto@kernel.org, jun.nakajima@intel.com, dave.hansen@intel.com, ak@linux.intel.com, david@redhat.com Subject: Re: [PATCH v5 06/13] KVM: Use kvm_userspace_memory_region_ext Message-ID: References: <20220310140911.50924-1-chao.p.peng@linux.intel.com> <20220310140911.50924-7-chao.p.peng@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220310140911.50924-7-chao.p.peng@linux.intel.com> X-Stat-Signature: rhdx3ik836773fgb3s8pb6uueqhy7r97 Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=Xyvle5xj; spf=pass (imf15.hostedemail.com: domain of seanjc@google.com designates 209.85.215.175 as permitted sender) smtp.mailfrom=seanjc@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 739B4A003A X-HE-Tag: 1648506421-572329 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: On Thu, Mar 10, 2022, Chao Peng wrote: > @@ -4476,14 +4477,23 @@ static long kvm_vm_ioctl(struct file *filp, > break; > } > case KVM_SET_USER_MEMORY_REGION: { > - struct kvm_userspace_memory_region kvm_userspace_mem; > + struct kvm_userspace_memory_region_ext region_ext; It's probably a good idea to zero initialize the full region to avoid consuming garbage stack data if there's a bug and an _ext field is accessed without first checking KVM_MEM_PRIVATE. I'm usually opposed to unnecessary initialization, but this seems like something we could screw up quite easily. > r = -EFAULT; > - if (copy_from_user(&kvm_userspace_mem, argp, > - sizeof(kvm_userspace_mem))) > + if (copy_from_user(®ion_ext, argp, > + sizeof(struct kvm_userspace_memory_region))) > goto out; > + if (region_ext.region.flags & KVM_MEM_PRIVATE) { > + int offset = offsetof( > + struct kvm_userspace_memory_region_ext, > + private_offset); > + if (copy_from_user(®ion_ext.private_offset, > + argp + offset, > + sizeof(region_ext) - offset)) In this patch, KVM_MEM_PRIVATE should result in an -EINVAL as it's not yet supported. Copying the _ext on KVM_MEM_PRIVATE belongs in the "Expose KVM_MEM_PRIVATE" patch. Mechnically, what about first reading flags via get_user(), and then doing a single copy_from_user()? It's technically more work in the common case, and requires an extra check to guard against TOCTOU attacks, but this isn't a fast path by any means and IMO the end result makes it easier to understand the relationship between KVM_MEM_PRIVATE and the two different structs. E.g. case KVM_SET_USER_MEMORY_REGION: { struct kvm_user_mem_region region; unsigned long size; u32 flags; memset(®ion, 0, sizeof(region)); r = -EFAULT; if (get_user(flags, (u32 __user *)(argp + offsetof(typeof(region), flags)))) goto out; if (flags & KVM_MEM_PRIVATE) size = sizeof(struct kvm_userspace_memory_region_ext); else size = sizeof(struct kvm_userspace_memory_region); if (copy_from_user(®ion, argp, size)) goto out; r = -EINVAL; if ((flags ^ region.flags) & KVM_MEM_PRIVATE) goto out; r = kvm_vm_ioctl_set_memory_region(kvm, ®ion); break; } > + goto out; > + } > > - r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem); > + r = kvm_vm_ioctl_set_memory_region(kvm, ®ion_ext); > break; > } > case KVM_GET_DIRTY_LOG: { > -- > 2.17.1 >