From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (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 7B76C1D539 for ; Mon, 30 Oct 2023 22:12:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="WmJRwsVA" Received: from mail-pf1-x449.google.com (mail-pf1-x449.google.com [IPv6:2607:f8b0:4864:20::449]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B41DEFE for ; Mon, 30 Oct 2023 15:12:12 -0700 (PDT) Received: by mail-pf1-x449.google.com with SMTP id d2e1a72fcca58-6be1065cc81so5043802b3a.3 for ; Mon, 30 Oct 2023 15:12:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1698703932; x=1699308732; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=g8DEvtikhnQyn+T5K55cZjm1cs6/IKtx/WCyiBBil5U=; b=WmJRwsVAjOdugnxCaIdp3UcES2KqSgFbohBM76jKC7YPTZJdwtQ2PJU+QhiSZRiH2N 5grrkTcJsGZVpzmuJiqBvCAH1jTwlEWNQ6KcmVPhvVQu+YoCWBCWzm6bIO9PqhzlbpZp bS6cnQ2aFt/5tjb0yB7qKOY3M/1aFkZWpPMzd1YNkwGkI5BJrTgc2Xx9Eat3tgF88QvR 0KnkZG31xaUlQfT7A+8SN8KfNPxTV7vbtZmhGawVrF4NEf/86WzkrpHQrHqu808T/NsV Hmzh/NgXh85ruWCCmnVbRbrEROpg/OPDFFG1M6s5VPyp+ehahaeFeXgpKRtJp/Y/So0q db0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698703932; x=1699308732; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=g8DEvtikhnQyn+T5K55cZjm1cs6/IKtx/WCyiBBil5U=; b=FHFqvUAgJO1hxiQSE9wAOkWOnAZVwSiw7UX8QhBMH/rP7rslr3EPESlSMy8yQsVyWy zDT7e7GSnuv+9I9QtAonEGwdbxb66WqDyLftDc5GofOq0pt9qsVv3ecmMb0k6SO/3QNv LSRqZUfUUPhW0TQEG33M6XIgFKplVInjyNv5ymtX4TQk4o3RTb31oLm9GEJGO48LjS8i vtLNI5f3a57LCLSskIaYCnHUFay5nTSX6HGIaFXWlXFiEcjXhs3umq+pQiFLwKz7pcYk tjx6JkFu0K5oYK5tPbT6V2uN7ZNntQK3DVDWr8anTSIpMKJgs/PjghXqrXw4mwc6u3BR eK3g== X-Gm-Message-State: AOJu0YwByTNntAfZhUt6eN0UE0UToK2pX6t1pKt8gg3FW/0yp2MH6zju YE1TpuUKcJkpkPmLeCognopqYIGKYoQ= X-Google-Smtp-Source: AGHT+IGGcxVYGAYGv3uCrE8wHX1TR4p8IURd9gUNgfJNwa+fo9vdIR8HGGPD/6eLST5HVrX+Zf4WM1nECms= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a05:6a00:1755:b0:68e:24fe:d9a with SMTP id j21-20020a056a00175500b0068e24fe0d9amr205380pfc.2.1698703932137; Mon, 30 Oct 2023 15:12:12 -0700 (PDT) Date: Mon, 30 Oct 2023 15:12:10 -0700 In-Reply-To: Precedence: bulk X-Mailing-List: kvm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20231027182217.3615211-1-seanjc@google.com> <20231027182217.3615211-9-seanjc@google.com> <211d093f-4023-4a39-a23f-6d8543512675@redhat.com> Message-ID: Subject: Re: [PATCH v13 08/35] KVM: Introduce KVM_SET_USER_MEMORY_REGION2 From: Sean Christopherson To: Paolo Bonzini Cc: Marc Zyngier , Oliver Upton , Huacai Chen , Michael Ellerman , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , 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" Content-Type: text/plain; charset="us-ascii" On Mon, Oct 30, 2023, Sean Christopherson wrote: > On Mon, Oct 30, 2023, Paolo Bonzini wrote: > > On 10/27/23 20:21, Sean Christopherson wrote: > > > > > > + if (ioctl == KVM_SET_USER_MEMORY_REGION) > > > + size = sizeof(struct kvm_userspace_memory_region); > > > > This also needs a memset(&mem, 0, sizeof(mem)), otherwise the out-of-bounds > > access of the commit message becomes a kernel stack read. > > Ouch. There's some irony. Might be worth doing memset(&mem, -1, sizeof(mem)) > though as '0' is a valid file descriptor and a valid file offset. > > > Probably worth adding a check on valid flags here. > > Definitely needed. There's a very real bug here. But rather than duplicate flags > checking or plumb @ioctl all the way to __kvm_set_memory_region(), now that we > have the fancy guard(mutex) and there are no internal calls to kvm_set_memory_region(), > what if we: > > 1. Acquire/release slots_lock in __kvm_set_memory_region() Gah, this won't work with x86's usage, which acquire slots_lock quite far away from __kvm_set_memory_region() in several places. The rest of the idea still works, kvm_vm_ioctl_set_memory_region() will just need to take slots_lock manually. > 2. Call kvm_set_memory_region() from x86 code for the internal memslots > 3. Disallow *any* flags for internal memslots > 4. Open code check_memory_region_flags in kvm_vm_ioctl_set_memory_region() > 5. Pass @ioctl to kvm_vm_ioctl_set_memory_region() and allow KVM_MEM_PRIVATE > only for KVM_SET_USER_MEMORY_REGION2