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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A9CADC3F6B0 for ; Tue, 16 Aug 2022 21:43:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237063AbiHPVnR (ORCPT ); Tue, 16 Aug 2022 17:43:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49084 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231594AbiHPVnN (ORCPT ); Tue, 16 Aug 2022 17:43:13 -0400 Received: from mail-pl1-x632.google.com (mail-pl1-x632.google.com [IPv6:2607:f8b0:4864:20::632]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C2AE71C5 for ; Tue, 16 Aug 2022 14:43:11 -0700 (PDT) Received: by mail-pl1-x632.google.com with SMTP id jm11so8151924plb.13 for ; Tue, 16 Aug 2022 14:43:11 -0700 (PDT) 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; bh=fHoaQuQyrNOHA1CxVanqB+3Y6dYUWxDy7qrwrkskMNM=; b=j03HDlXVQVJ9go7wU3LUhqLEtXGqZRIIx4a+gV3xfUzmx9FveQlrGHP0cTzS7hAEtA 0AjKIumdLifjWu/ZY/c2QAI61e/LU4mar+fa2w3AODG4eMxptX6kBdFI8h73MyZX6I8g rJgjPVNJEVsU/H9aK8ViD/wvirZ2qf7HdntLAv7f7S0V9YSuVPBZbiDRJZdY4e2oqasO SyFyFfJuGeKSqFv7/fWyEYC/o4XYjiflNKFmwj/fUUkxdY3TpwUBwjiEgov0is4uN1oH Hx3EgJthTnCPgv9hmhQqqeArchhQ2hiGmmnGEJi8QkErohO2PFDJ2+rlAOU/8AHNBiB7 V+Rw== 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; bh=fHoaQuQyrNOHA1CxVanqB+3Y6dYUWxDy7qrwrkskMNM=; b=HvvITiBj/ubXgcU7WZkoTg5srX7uDmxAIaG79ryrtpNLy/GuWS0tbTnXEyfmAku53P ELcoDyjnNG070NQNAoa6YneU/VhPZl80xOTOusO+hItElsq7mEmHLc6UoXrbHilz1ude HI8mEwOZTAPqiAsQ16RnrzCSE3/R5/HPXZ82z7ELCRd7257BD19xW/YNcnKl3Dxl3G2A R7bk+tr7RhP/N/bsaM3+LWqd0wt+DJrWbIJqefxO8i4JTmrknGtmH6RSc+jeqlDshFQC aJa7SO6qqWBtydcO5FyrHyAhzTp5cuoih+ZPsJ2Iq3S6x4uZp97f0gMNA8EfYkBJPdU7 XVFQ== X-Gm-Message-State: ACgBeo0oB5L3NXBbLyeEip2z+v8WRS/sK5vP/SYCVnlt/TA/7N3Ecxug H4Yk/uP0qKGmyN3BbbtTQc18qw== X-Google-Smtp-Source: AA6agR4aqvIq74t1vyp7m/HiiEbuxiLefbEN5Ecig7re+ePSp2kvKMMY09q7oON6KvCRY2iBzeP+Cw== X-Received: by 2002:a17:902:d2d1:b0:16e:eeac:29ab with SMTP id n17-20020a170902d2d100b0016eeeac29abmr24111405plc.125.1660686191203; Tue, 16 Aug 2022 14:43:11 -0700 (PDT) Received: from google.com (7.104.168.34.bc.googleusercontent.com. [34.168.104.7]) by smtp.gmail.com with ESMTPSA id m7-20020a1709026bc700b0016bf5557690sm9533377plt.4.2022.08.16.14.43.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Aug 2022 14:43:10 -0700 (PDT) Date: Tue, 16 Aug 2022 21:43:07 +0000 From: Sean Christopherson To: David Matlack Cc: Paolo Bonzini , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, syzbot+744e173caec2e1627ee0@syzkaller.appspotmail.com, Oliver Upton Subject: Re: [PATCH 2/3] KVM: Unconditionally get a ref to /dev/kvm module when creating a VM Message-ID: References: <20220816053937.2477106-1-seanjc@google.com> <20220816053937.2477106-3-seanjc@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Aug 16, 2022, David Matlack wrote: > On Tue, Aug 16, 2022 at 05:39:36AM +0000, Sean Christopherson wrote: > > Unconditionally get a reference to the /dev/kvm module when creating a VM > > instead of using try_get_module(), which will fail if the module is in > > the process of being forcefully unloaded. The error handling when > > try_get_module() fails doesn't properly unwind all that has been done, > > e.g. doesn't call kvm_arch_pre_destroy_vm() and doesn't remove the VM > > from the global list. Not removing VMs from the global list tends to be > > fatal, e.g. leads to use-after-free explosions. > > > > The obvious alternative would be to add proper unwinding, but the > > justification for using try_get_module(), "rmmod --wait", is completely > > bogus as support for "rmmod --wait", i.e. delete_module() without > > O_NONBLOCK, was removed by commit 3f2b9c9cdf38 ("module: remove rmmod > > --wait option.") nearly a decade ago. > > Ah! include/linux/module.h may also need a cleanup then. The comment > above __module_get() explicitly mentions "rmmod --wait", which is what > led me to use try_module_get() for commit 5f6de5cbebee ("KVM: Prevent > module exit until all VMs are freed"). Ugh, I didn't see that one. The whole thing is a mess. try_module_get() also has a comment (just below the "rmmod --wait" comment) saying that it's the one true way of doing things, but that's at best misleading for cases like this where a module is taking a reference of _itself_. The man pages are also woefully out of date :-/