From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Christopherson Date: Tue, 12 Dec 2023 18:22:00 -0800 Subject: [PATCH 05/26] vfio: KVM: Pass get/put helpers from KVM to VFIO, don't do circular lookup In-Reply-To: <20231203140756.GI1489931@ziepe.ca> References: <20230916003118.2540661-1-seanjc@google.com> <20230916003118.2540661-6-seanjc@google.com> <20230918152110.GI13795@ziepe.ca> <20230918160258.GL13795@ziepe.ca> <20231203140756.GI1489931@ziepe.ca> 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 Sun, Dec 03, 2023, Jason Gunthorpe wrote: > On Fri, Dec 01, 2023 at 04:51:55PM -0800, Sean Christopherson wrote: > > > There's one more wrinkle: this patch is buggy in that it doesn't ensure the liveliness > > of KVM-the-module, i.e. nothing prevents userspace from unloading kvm.ko while VFIO > > still holds a reference to a kvm structure, and so invoking ->put_kvm() could jump > > into freed code. To fix that, KVM would also need to pass along a module pointer :-( > > Maybe we should be refcounting the struct file not the struct kvm? > > Then we don't need special helpers and it keeps the module alive correctly. Huh. It took my brain a while to catch up, but this seems comically obvious in hindsight. I *love* this approach, both conceptually and from a code perspective. Handing VFIO (and any other external entities) a file makes it so that KVM effectively interacts with users via files, regardless of whether the user lives in userspace or the kernel. That makes it easier to reason about the safety of operations, e.g. in addition to ensuring KVM-the-module is pinned, having a file pointer allows KVM to verify that the incoming pointer does indeed represent a VM. Which isn't necessary by any means, but it's a nice sanity check. >From a code perspective, it's far cleaner than manually grabbing module references, and having only a file pointers makes it a wee bit harder for non-KVM code to poke into KVM, e.g. keeps us honest. Assuming nothing blows up in testing, I'll go this route for v2. Thanks! From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f73.google.com (mail-pj1-f73.google.com [209.85.216.73]) (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 706BFECE for ; Wed, 13 Dec 2023 02:22:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="daZu9QmO" Received: by mail-pj1-f73.google.com with SMTP id 98e67ed59e1d1-2869cdac540so3623363a91.0 for ; Tue, 12 Dec 2023 18:22:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702434122; x=1703038922; darn=lists.linux.dev; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=H3dbLwG3pwrVNc+flPqWL0ROEahdeYkxOItBObS2igQ=; b=daZu9QmOMzQBgTf9EodVE8CRM8x8TwlfhIgKVetVzpil60Mh/xpkwD8lnj33VdY+Up TYG7Dlcc7q6zQg0L+xVZINSyGOBURwFgDEA27uoSSJ8Ob5a5M1kkRmECfIhwiyqmaJmo QN2Fy2Q7N7Te5L3QsSX65sotDjA81wzkqvXsZKzr8OKmYuMYGFqI04knICk8MKcDvGCh 1vTjvG9PBBvuNGJ8rM5YElTmDCZxQHWnkfsMcaXp9AwGxszoNr7ZyHiNOrM8QwQA7SRv V3tWOD3CY5DFW5DOJ+UYnPPJ+G3+v1o+IUtKYy4FKzEZcRBYg+/SlZn5sbdPQzJCq262 m/Yw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702434122; x=1703038922; 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=H3dbLwG3pwrVNc+flPqWL0ROEahdeYkxOItBObS2igQ=; b=cZKUgkpdogb2Emv/8/BSUZk8PtquJD81kfyC87kTCCTWrGg2nBLWA3F1Pr3srG4PQO FJlFlx0bDpab/9X3cDzBK6dAvpfKR3LMKI7zCrpQrzDb7Smwo3N15DxQdqziiLkc3KoR zjFNQ21UMU/xRxhbktNakBs3xgjFMf+GcsuiW0JMX3ZsXYS7WoUBehLzdtvv8fv4Jqta 51A3XrvXAEBlf9VHcGTNvgEaqNBZaBbSKuPLGK7wLWxzXFEfbgg+yOt6bjMDs4qyRJ6j kYbUvDLj8JfOVOx/4Wvo05Y/4DbDilbemda2zBk5434O/+J9K3u9pvLyKuWEr7RKL+SK vihQ== X-Gm-Message-State: AOJu0YwPSX0J3/997qd7SLIz6AOIYaRgHxZTEbnJhPb9KmVU5h//N8M9 XQ13Hwcou0PW+BqFhy3q5OnvgQaEI80= X-Google-Smtp-Source: AGHT+IEMdNbXZWy9ISUMeWXjYiRzXRSqXuBEtLtB2quA6rPLGk41btEuk2vriof3H/B+MEphfzV6B3hdgbQ= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:903:41c9:b0:1d0:cd87:64dd with SMTP id u9-20020a17090341c900b001d0cd8764ddmr52067ple.3.1702434121579; Tue, 12 Dec 2023 18:22:01 -0800 (PST) Date: Tue, 12 Dec 2023 18:22:00 -0800 In-Reply-To: <20231203140756.GI1489931@ziepe.ca> Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20230916003118.2540661-1-seanjc@google.com> <20230916003118.2540661-6-seanjc@google.com> <20230918152110.GI13795@ziepe.ca> <20230918160258.GL13795@ziepe.ca> <20231203140756.GI1489931@ziepe.ca> Message-ID: Subject: Re: [PATCH 05/26] vfio: KVM: Pass get/put helpers from KVM to VFIO, don't do circular lookup From: Sean Christopherson To: Jason Gunthorpe Cc: Catalin Marinas , Will Deacon , Marc Zyngier , Oliver Upton , Huacai Chen , Michael Ellerman , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, Peter Zijlstra , Arnaldo Carvalho de Melo , Paolo Bonzini , Tony Krowiak , Halil Pasic , Jason Herne , Harald Freudenberger , Alex Williamson , Andy Lutomirski , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-mips@vger.kernel.org, kvm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, Anish Ghulati , Venkatesh Srinivas , Andrew Thornton Content-Type: text/plain; charset="us-ascii" On Sun, Dec 03, 2023, Jason Gunthorpe wrote: > On Fri, Dec 01, 2023 at 04:51:55PM -0800, Sean Christopherson wrote: > > > There's one more wrinkle: this patch is buggy in that it doesn't ensure the liveliness > > of KVM-the-module, i.e. nothing prevents userspace from unloading kvm.ko while VFIO > > still holds a reference to a kvm structure, and so invoking ->put_kvm() could jump > > into freed code. To fix that, KVM would also need to pass along a module pointer :-( > > Maybe we should be refcounting the struct file not the struct kvm? > > Then we don't need special helpers and it keeps the module alive correctly. Huh. It took my brain a while to catch up, but this seems comically obvious in hindsight. I *love* this approach, both conceptually and from a code perspective. Handing VFIO (and any other external entities) a file makes it so that KVM effectively interacts with users via files, regardless of whether the user lives in userspace or the kernel. That makes it easier to reason about the safety of operations, e.g. in addition to ensuring KVM-the-module is pinned, having a file pointer allows KVM to verify that the incoming pointer does indeed represent a VM. Which isn't necessary by any means, but it's a nice sanity check. >From a code perspective, it's far cleaner than manually grabbing module references, and having only a file pointers makes it a wee bit harder for non-KVM code to poke into KVM, e.g. keeps us honest. Assuming nothing blows up in testing, I'll go this route for v2. Thanks! 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 B038EC4332F for ; Wed, 13 Dec 2023 02:22:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:Message-ID: References:Mime-Version:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=DHtVzIAkgPzctaH1Zyhz7X2IIDCOsPSQUcL02/389pM=; b=bZXd5EnJ8cRzMG7sGhn7noYH+i t7wPX5UPoU7BsqVCNZUZtwlfP8fvyQm6qwLxpaOO2FVxF2OFu5hTexqGMpDUANPO61Z979uQaajY5 f5k40vppw1iz6pI4+PuuRlCrmQc5l4O910t1ig9pNA6U+yhMTEAwsgGMYPbU7HFr8jNwyoa6tN4PO F2Ve5fyI/zJVgwDfXAgnfBLoBORVPPDsEK7fstIwaV7om1OrVqvOxSHtCaLHgqz7a6optQtFH5+c1 ha1Q/+qBCcJEqcNkte1jyZB2sxT+eg8lnKQ8YN1t/WKg/y2cB0NciDhhWKiobfbXfAZV91LzlracM YFq6Fkxw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rDEsx-00DOXU-24; Wed, 13 Dec 2023 02:22:07 +0000 Received: from mail-pj1-x1049.google.com ([2607:f8b0:4864:20::1049]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rDEsu-00DOVx-2r for linux-riscv@lists.infradead.org; Wed, 13 Dec 2023 02:22:06 +0000 Received: by mail-pj1-x1049.google.com with SMTP id 98e67ed59e1d1-28ae055169eso111585a91.2 for ; Tue, 12 Dec 2023 18:22:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702434121; x=1703038921; darn=lists.infradead.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=H3dbLwG3pwrVNc+flPqWL0ROEahdeYkxOItBObS2igQ=; b=h0wBYoiN9XMbOC3aEqFkTuow3aXaQErOo8Z4+7HoZwbNFXCVOCemSZ8EmHJ2LPwDyF 8zoQn2nmbbVsnqYyVZ9HkfAP3Fb+ucuvvwVObN2bAT+yJcy7mjDL1wm9PQ9Ik93xd5jq IQbtfAeu90ECe1qYPcw/LhVrBIeFS9bMKBKMgIzVchKuI2XTG2AluEESaUGsQ9wtPe7V jATz5Qdn22IsZ/jxhcnVDy2zJaHD8rTSq11ayvcbzFAhHnpLrpWnIRNaA9qHA3oLel1X d2iBNSyLLjWatCq/Y2ILXjlIbRtJ125iQw8vUEOvykjeOsmCVO9ar7BjhUxwnnsCQeMT 1npQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702434121; x=1703038921; 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=H3dbLwG3pwrVNc+flPqWL0ROEahdeYkxOItBObS2igQ=; b=onn4pCjkHeQ67BQBxmZtQQaf54bl1a6BYe2XXDG1OSouLT4grQL7wNoFSyn45dikSF z5nUFfJRX7uG4NR7bPVn1wbnAxRg1Y2TvahXwpXQ+3fRy1Iggl+l7+SYw1+8xV4QsCei 4OoBB5HtxRWEsUkhd+R3pU0WywH96poHFU27b9pNPmB28VXKLouMjWDbfLfKK8BXeFOw JS0v7hH6Q/j1mhoKDdX2zbR0mop+hZ1ES7qsiDspxdRMpWoW347NwPJV04WHq49Rw1gq BdnAsPt2SAbiHxGj65f2KhkV1mnLE6JNJJ1lDksftAxWQcm3yDpvDvEZOuQ+4+r02Txe 2ZVA== X-Gm-Message-State: AOJu0YzSH1KFsD4ycJ5uz2S0qqCGDAkfhqHmP496TZRWVtCr5EDVtgTS 9O0DMjAsHIEMHAkoG3WdHxRdiNEk8nU= X-Google-Smtp-Source: AGHT+IEMdNbXZWy9ISUMeWXjYiRzXRSqXuBEtLtB2quA6rPLGk41btEuk2vriof3H/B+MEphfzV6B3hdgbQ= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:903:41c9:b0:1d0:cd87:64dd with SMTP id u9-20020a17090341c900b001d0cd8764ddmr52067ple.3.1702434121579; Tue, 12 Dec 2023 18:22:01 -0800 (PST) Date: Tue, 12 Dec 2023 18:22:00 -0800 In-Reply-To: <20231203140756.GI1489931@ziepe.ca> Mime-Version: 1.0 References: <20230916003118.2540661-1-seanjc@google.com> <20230916003118.2540661-6-seanjc@google.com> <20230918152110.GI13795@ziepe.ca> <20230918160258.GL13795@ziepe.ca> <20231203140756.GI1489931@ziepe.ca> Message-ID: Subject: Re: [PATCH 05/26] vfio: KVM: Pass get/put helpers from KVM to VFIO, don't do circular lookup From: Sean Christopherson To: Jason Gunthorpe Cc: Catalin Marinas , Will Deacon , Marc Zyngier , Oliver Upton , Huacai Chen , Michael Ellerman , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, Peter Zijlstra , Arnaldo Carvalho de Melo , Paolo Bonzini , Tony Krowiak , Halil Pasic , Jason Herne , Harald Freudenberger , Alex Williamson , Andy Lutomirski , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-mips@vger.kernel.org, kvm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, Anish Ghulati , Venkatesh Srinivas , Andrew Thornton X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231212_182204_949800_6CB49F63 X-CRM114-Status: GOOD ( 16.28 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Sun, Dec 03, 2023, Jason Gunthorpe wrote: > On Fri, Dec 01, 2023 at 04:51:55PM -0800, Sean Christopherson wrote: > > > There's one more wrinkle: this patch is buggy in that it doesn't ensure the liveliness > > of KVM-the-module, i.e. nothing prevents userspace from unloading kvm.ko while VFIO > > still holds a reference to a kvm structure, and so invoking ->put_kvm() could jump > > into freed code. To fix that, KVM would also need to pass along a module pointer :-( > > Maybe we should be refcounting the struct file not the struct kvm? > > Then we don't need special helpers and it keeps the module alive correctly. Huh. It took my brain a while to catch up, but this seems comically obvious in hindsight. I *love* this approach, both conceptually and from a code perspective. Handing VFIO (and any other external entities) a file makes it so that KVM effectively interacts with users via files, regardless of whether the user lives in userspace or the kernel. That makes it easier to reason about the safety of operations, e.g. in addition to ensuring KVM-the-module is pinned, having a file pointer allows KVM to verify that the incoming pointer does indeed represent a VM. Which isn't necessary by any means, but it's a nice sanity check. >From a code perspective, it's far cleaner than manually grabbing module references, and having only a file pointers makes it a wee bit harder for non-KVM code to poke into KVM, e.g. keeps us honest. Assuming nothing blows up in testing, I'll go this route for v2. Thanks! _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv 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 060EAC4167D for ; Wed, 13 Dec 2023 02:22:58 +0000 (UTC) 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=20230601 header.b=pjdRffJw; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4SqfQn2307z3cRJ for ; Wed, 13 Dec 2023 13:22:57 +1100 (AEDT) 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=20230601 header.b=pjdRffJw; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=flex--seanjc.bounces.google.com (client-ip=2607:f8b0:4864:20::1049; helo=mail-pj1-x1049.google.com; envelope-from=3srv5zqykdio6so1xqu22uzs.q20zw18b33q-rs9zw676.2dzop6.25u@flex--seanjc.bounces.google.com; receiver=lists.ozlabs.org) Received: from mail-pj1-x1049.google.com (mail-pj1-x1049.google.com [IPv6:2607:f8b0:4864:20::1049]) (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 4SqfPr0yzqz3bX9 for ; Wed, 13 Dec 2023 13:22:06 +1100 (AEDT) Received: by mail-pj1-x1049.google.com with SMTP id 98e67ed59e1d1-28ae055169eso111586a91.2 for ; Tue, 12 Dec 2023 18:22:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702434122; x=1703038922; darn=lists.ozlabs.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=H3dbLwG3pwrVNc+flPqWL0ROEahdeYkxOItBObS2igQ=; b=pjdRffJwjugT+tlgaE1SE+3qb3h44q6XLCeUscKsqyMusP8qKln4mfLW9ib6uEOzeC eOmZGVGkivT4SRK5vodO/kE7Mgm31Fk3FwUVYrBp1xhCdVTtENAN0nFXB3KmnXGkgToN mWRYjPKKXX56K1FKtM0DetdU3UzYtF7pwFhjUPCh0ZPBA5JuebSasF20Qgbba5K+nlB8 ywToeQOVSDL8wpTIyTDkhEkBNzeiLOsrGATZb9yWgXNQjVOV82gjQBLmqoAbrKbLwe7G iYFZrqhurZhg3RGw4jAvKWwVLymfLStpmnXHpQSzffMZx+76ujdqtaTbA8T3DTXUtsgu I1bg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702434122; x=1703038922; 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=H3dbLwG3pwrVNc+flPqWL0ROEahdeYkxOItBObS2igQ=; b=CWBPNShM+IAgQJLeF5t6dn0zrVCnJzbLkSLBlejP1ykzaZINwzU17Tb1mdv+Wxe64y L64rFoGJJ8P9mFufmiHGg4Ub7sbmWGrByfqKe+HuAZG7LD5DJYzgrT+Ld2hgs9+PNbsQ PqBeC4X/85SLDlMw8hoxw1pNO60ACFzVT0ngTo0sjz6eC4+yEl//7/KxAMcEwVvOl3ei lrPQ1F/ZMAXFpYAw2ZDeFhOVuYD3TCHE3Jp/JS8fMippmpyHllyC3CrJyFHfhETZsrXu ZVo0+btrfInmfy5YmAHarIjAdML+WDlkD2sHLFgtmdtiCKqopsjqaCKXnNQ4fl1SOM6K m45A== X-Gm-Message-State: AOJu0Yxb7rWJ88l+/+mcJguptUxSIs6IE2k/90D9dfgG9emLgXdEB4hB lPnXqa05Rt01R1rnwKtY6ZPiHHR8Pb4= X-Google-Smtp-Source: AGHT+IEMdNbXZWy9ISUMeWXjYiRzXRSqXuBEtLtB2quA6rPLGk41btEuk2vriof3H/B+MEphfzV6B3hdgbQ= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:903:41c9:b0:1d0:cd87:64dd with SMTP id u9-20020a17090341c900b001d0cd8764ddmr52067ple.3.1702434121579; Tue, 12 Dec 2023 18:22:01 -0800 (PST) Date: Tue, 12 Dec 2023 18:22:00 -0800 In-Reply-To: <20231203140756.GI1489931@ziepe.ca> Mime-Version: 1.0 References: <20230916003118.2540661-1-seanjc@google.com> <20230916003118.2540661-6-seanjc@google.com> <20230918152110.GI13795@ziepe.ca> <20230918160258.GL13795@ziepe.ca> <20231203140756.GI1489931@ziepe.ca> Message-ID: Subject: Re: [PATCH 05/26] vfio: KVM: Pass get/put helpers from KVM to VFIO, don't do circular lookup From: Sean Christopherson To: Jason Gunthorpe Content-Type: text/plain; charset="us-ascii" 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: x86@kernel.org, kvm@vger.kernel.org, Peter Zijlstra , Catalin Marinas , Dave Hansen , linux-kernel@vger.kernel.org, Alexander Gordeev , Claudio Imbrenda , Will Deacon , linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, Janosch Frank , Harald Freudenberger , Marc Zyngier , Huacai Chen , Halil Pasic , Andrew Thornton , Ingo Molnar , Christian Borntraeger , Jason Herne , Albert Ou , Vasily Gorbik , Venkatesh Srinivas , Heiko Carstens , Arnaldo Carvalho de Melo , Alex Williamson , Borislav Petkov , And y Lutomirski , Paul Walmsley , kvmarm@lists.linux.dev, Thomas Gleixner , linux-arm-kernel@lists.infradead.org, Tony Krowiak , Anish Ghulati , linux-mips@vger.kernel.org, Oliver Upton , linux-perf-users@vger.kernel.org, Palmer Dabbelt , kvm-riscv@lists.infradead.org, Anup Patel , Paolo Bonzini , linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Sun, Dec 03, 2023, Jason Gunthorpe wrote: > On Fri, Dec 01, 2023 at 04:51:55PM -0800, Sean Christopherson wrote: > > > There's one more wrinkle: this patch is buggy in that it doesn't ensure the liveliness > > of KVM-the-module, i.e. nothing prevents userspace from unloading kvm.ko while VFIO > > still holds a reference to a kvm structure, and so invoking ->put_kvm() could jump > > into freed code. To fix that, KVM would also need to pass along a module pointer :-( > > Maybe we should be refcounting the struct file not the struct kvm? > > Then we don't need special helpers and it keeps the module alive correctly. Huh. It took my brain a while to catch up, but this seems comically obvious in hindsight. I *love* this approach, both conceptually and from a code perspective. Handing VFIO (and any other external entities) a file makes it so that KVM effectively interacts with users via files, regardless of whether the user lives in userspace or the kernel. That makes it easier to reason about the safety of operations, e.g. in addition to ensuring KVM-the-module is pinned, having a file pointer allows KVM to verify that the incoming pointer does indeed represent a VM. Which isn't necessary by any means, but it's a nice sanity check. >From a code perspective, it's far cleaner than manually grabbing module references, and having only a file pointers makes it a wee bit harder for non-KVM code to poke into KVM, e.g. keeps us honest. Assuming nothing blows up in testing, I'll go this route for v2. Thanks! 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 C677FC4167D for ; Wed, 13 Dec 2023 02:22:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:Message-ID: References:Mime-Version:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=6eYhKoEs6iAGEn9Mo2XEg6D78/YM9C6VTGW6OCX5HVE=; b=MzKFNj61Lgj1d+WlEMp5OyJEzB Ch4S5mnIQvJgH2GabRzimZWfa9wa9qz3/FlZFnbtM3eYh0uEQGvhl7xGVegtuGMNGmcwuhRWnncOx /amucTm8gnIXh5toWr6JgvWmvNcTT4AYKklPOVfI5VQiDzFUM7cOkahs2tqgz8H64DRDv8lT5omXA WsyAI5mnv3eD4XCKp8jjnfw1bImKLkvaIkJHddrQNWjCpw7hHLTmon+MyMxEVHQ3f+BxT+jBc4QFk P1DAseNo3E/0vP2nuqet9uDl12VHK//zFA5ciJ/p4ITnUTeyuN/CS5b8i5ppY8mdbVP/N7A7WGlVT C/y8S8rQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rDEsy-00DOY1-21; Wed, 13 Dec 2023 02:22:08 +0000 Received: from mail-pj1-x104a.google.com ([2607:f8b0:4864:20::104a]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rDEsv-00DOVw-14 for linux-arm-kernel@lists.infradead.org; Wed, 13 Dec 2023 02:22:07 +0000 Received: by mail-pj1-x104a.google.com with SMTP id 98e67ed59e1d1-28a30542c37so2284271a91.1 for ; Tue, 12 Dec 2023 18:22:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702434121; x=1703038921; darn=lists.infradead.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=H3dbLwG3pwrVNc+flPqWL0ROEahdeYkxOItBObS2igQ=; b=h0wBYoiN9XMbOC3aEqFkTuow3aXaQErOo8Z4+7HoZwbNFXCVOCemSZ8EmHJ2LPwDyF 8zoQn2nmbbVsnqYyVZ9HkfAP3Fb+ucuvvwVObN2bAT+yJcy7mjDL1wm9PQ9Ik93xd5jq IQbtfAeu90ECe1qYPcw/LhVrBIeFS9bMKBKMgIzVchKuI2XTG2AluEESaUGsQ9wtPe7V jATz5Qdn22IsZ/jxhcnVDy2zJaHD8rTSq11ayvcbzFAhHnpLrpWnIRNaA9qHA3oLel1X d2iBNSyLLjWatCq/Y2ILXjlIbRtJ125iQw8vUEOvykjeOsmCVO9ar7BjhUxwnnsCQeMT 1npQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702434121; x=1703038921; 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=H3dbLwG3pwrVNc+flPqWL0ROEahdeYkxOItBObS2igQ=; b=Z0tC6TvO9BiZRtoT+L8iMcHY3WV6f+WtmxE+c+8io/cHmkIKeGQpdQfTTmxj5od40H NXFdLt/tlwH2Oq5pfT8Hm+X7oLnbqcAe5iBRUE7ayVy1A9J8KFkqNx2XAmV+gUm+ZiKd sol6J/rbmcHgISKoC2+HwoVe3H+sDArGzvCnL182RpZ7iWUSsa1vkf19Q2V+RnLX3LCM 63cIP1lestpLQMfsyoXeFgdZ3xBeYlBDWIKjRWOWn2k7cwXXBVLnN85khZ3EPGVMd2gR OCPNO1k8Hvy5HZSX1vRDkHajCAn05cRQE61Ae6Z2fhQSVOq327GGHbTqKhenYYITSOlK LbUA== X-Gm-Message-State: AOJu0YxpKV4/q3J4EAFGHOp2x/TmiIsK4Pufof+EedLqtJlWA8xKu1Xx dgI6KZ2+0X98h3Mfrqj6GGeRaVKfcFA= X-Google-Smtp-Source: AGHT+IEMdNbXZWy9ISUMeWXjYiRzXRSqXuBEtLtB2quA6rPLGk41btEuk2vriof3H/B+MEphfzV6B3hdgbQ= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:903:41c9:b0:1d0:cd87:64dd with SMTP id u9-20020a17090341c900b001d0cd8764ddmr52067ple.3.1702434121579; Tue, 12 Dec 2023 18:22:01 -0800 (PST) Date: Tue, 12 Dec 2023 18:22:00 -0800 In-Reply-To: <20231203140756.GI1489931@ziepe.ca> Mime-Version: 1.0 References: <20230916003118.2540661-1-seanjc@google.com> <20230916003118.2540661-6-seanjc@google.com> <20230918152110.GI13795@ziepe.ca> <20230918160258.GL13795@ziepe.ca> <20231203140756.GI1489931@ziepe.ca> Message-ID: Subject: Re: [PATCH 05/26] vfio: KVM: Pass get/put helpers from KVM to VFIO, don't do circular lookup From: Sean Christopherson To: Jason Gunthorpe Cc: Catalin Marinas , Will Deacon , Marc Zyngier , Oliver Upton , Huacai Chen , Michael Ellerman , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, Peter Zijlstra , Arnaldo Carvalho de Melo , Paolo Bonzini , Tony Krowiak , Halil Pasic , Jason Herne , Harald Freudenberger , Alex Williamson , Andy Lutomirski , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-mips@vger.kernel.org, kvm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, Anish Ghulati , Venkatesh Srinivas , Andrew Thornton X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231212_182205_368658_3843C25A X-CRM114-Status: GOOD ( 17.88 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sun, Dec 03, 2023, Jason Gunthorpe wrote: > On Fri, Dec 01, 2023 at 04:51:55PM -0800, Sean Christopherson wrote: > > > There's one more wrinkle: this patch is buggy in that it doesn't ensure the liveliness > > of KVM-the-module, i.e. nothing prevents userspace from unloading kvm.ko while VFIO > > still holds a reference to a kvm structure, and so invoking ->put_kvm() could jump > > into freed code. To fix that, KVM would also need to pass along a module pointer :-( > > Maybe we should be refcounting the struct file not the struct kvm? > > Then we don't need special helpers and it keeps the module alive correctly. Huh. It took my brain a while to catch up, but this seems comically obvious in hindsight. I *love* this approach, both conceptually and from a code perspective. Handing VFIO (and any other external entities) a file makes it so that KVM effectively interacts with users via files, regardless of whether the user lives in userspace or the kernel. That makes it easier to reason about the safety of operations, e.g. in addition to ensuring KVM-the-module is pinned, having a file pointer allows KVM to verify that the incoming pointer does indeed represent a VM. Which isn't necessary by any means, but it's a nice sanity check. >From a code perspective, it's far cleaner than manually grabbing module references, and having only a file pointers makes it a wee bit harder for non-KVM code to poke into KVM, e.g. keeps us honest. Assuming nothing blows up in testing, I'll go this route for v2. Thanks! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel