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 40FEED3A67B for ; Tue, 29 Oct 2024 18:46:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=zDxtU9YNz9xnWBQRtb5L5rJFy6fv+lmyeyTlVJB9PJw=; b=i3j9TpdUNhlrF6D7bQ7s1mvOe9 IOaSuYXLtMEEgWKN76MeruXP2OG8onWo4CizvSmyaSQyKttD+GhE/E1GBxqgxlw0MVNKG/A4/8EsZ 8g0EMpSBtlDJ6PrYI2JUMyhMw+Ju23MkStUUmLXD2OuuZ7Mrvvh5mYvSRT6HMezz1SBDgStnuBAls Qt02GrWcEowpfqFd/WU6Sf7dDLdsFIKyVvaOHiqEMJNBixiEqMkAObSakxxge/Zl1liaABHz/tYpz uEg/zq7bIwFJUL4kDMj7bnCh8OoWsdxlPziTc7hXo2O4TAOklcnflESKDoSQxCY4L03QkwQSBtFEd lTTpP/pA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t5rDz-0000000FWuJ-3Dpq; Tue, 29 Oct 2024 18:45:51 +0000 Received: from nyc.source.kernel.org ([2604:1380:45d1:ec00::3]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1t5p4B-0000000F74h-1SUF for linux-arm-kernel@lists.infradead.org; Tue, 29 Oct 2024 16:27:36 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id A0199A4300A; Tue, 29 Oct 2024 16:25:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id DF11EC4CEE4; Tue, 29 Oct 2024 16:27:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1730219253; bh=Z3N4p1dwXhBN00KOleOrbra5ELWKA6izVcLKfEwUA6c=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=urbEN6hNzPqi50TUyCRNw61epjyTQT3YPQmW5yrOe5jO+DwS/+lgpRI2cqFKO9aG6 Tpj1T/xg6ZozpcDEl7bBQMK2X7sOdaEBneyvCIyXryQOP++9U80LeA7l42oOj+pkT8 6cE/IqUz/1R4Ag4cqlzcMruzENyFyNbd7MfJscoCVnAKwYUVB84oKPPa7Ql16tXKUK z9Y+Bri390dmftkG15PvWU+h0y9tA1SDDv09NpRhzJigMSUi0Lmz8GfSzztArAJAa3 1DhC4k7vUUVWQEbnEA5YIMAoS1pM04G/7l5OlC8meIxRBb4YcnT/VAZUnRNMueaHZ+ GKfjubpSsnUfw== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1t5p47-007z9Y-LO; Tue, 29 Oct 2024 16:27:31 +0000 Date: Tue, 29 Oct 2024 16:27:31 +0000 Message-ID: <868qu63mdo.wl-maz@kernel.org> From: Marc Zyngier To: Raghavendra Rao Ananta Cc: Oliver Upton , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, stable@vger.kernel.org, syzbot Subject: Re: [PATCH v2] KVM: arm64: Get rid of userspace_irqchip_in_use In-Reply-To: <20241028234533.942542-1-rananta@google.com> References: <20241028234533.942542-1-rananta@google.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.4 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: rananta@google.com, oliver.upton@linux.dev, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, stable@vger.kernel.org, syzkaller@googlegroups.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241029_092735_536505_2DB1EF4F X-CRM114-Status: GOOD ( 21.05 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 28 Oct 2024 23:45:33 +0000, Raghavendra Rao Ananta wrote: > > Improper use of userspace_irqchip_in_use led to syzbot hitting the > following WARN_ON() in kvm_timer_update_irq(): > > WARNING: CPU: 0 PID: 3281 at arch/arm64/kvm/arch_timer.c:459 > kvm_timer_update_irq+0x21c/0x394 > Call trace: > kvm_timer_update_irq+0x21c/0x394 arch/arm64/kvm/arch_timer.c:459 > kvm_timer_vcpu_reset+0x158/0x684 arch/arm64/kvm/arch_timer.c:968 > kvm_reset_vcpu+0x3b4/0x560 arch/arm64/kvm/reset.c:264 > kvm_vcpu_set_target arch/arm64/kvm/arm.c:1553 [inline] > kvm_arch_vcpu_ioctl_vcpu_init arch/arm64/kvm/arm.c:1573 [inline] > kvm_arch_vcpu_ioctl+0x112c/0x1b3c arch/arm64/kvm/arm.c:1695 > kvm_vcpu_ioctl+0x4ec/0xf74 virt/kvm/kvm_main.c:4658 > vfs_ioctl fs/ioctl.c:51 [inline] > __do_sys_ioctl fs/ioctl.c:907 [inline] > __se_sys_ioctl fs/ioctl.c:893 [inline] > __arm64_sys_ioctl+0x108/0x184 fs/ioctl.c:893 > __invoke_syscall arch/arm64/kernel/syscall.c:35 [inline] > invoke_syscall+0x78/0x1b8 arch/arm64/kernel/syscall.c:49 > el0_svc_common+0xe8/0x1b0 arch/arm64/kernel/syscall.c:132 > do_el0_svc+0x40/0x50 arch/arm64/kernel/syscall.c:151 > el0_svc+0x54/0x14c arch/arm64/kernel/entry-common.c:712 > el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:730 > el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:598 > > The following sequence led to the scenario: > - Userspace creates a VM and a vCPU. > - The vCPU is initialized with KVM_ARM_VCPU_PMU_V3 during > KVM_ARM_VCPU_INIT. > - Without any other setup, such as vGIC or vPMU, userspace issues > KVM_RUN on the vCPU. Since the vPMU is requested, but not setup, > kvm_arm_pmu_v3_enable() fails in kvm_arch_vcpu_run_pid_change(). > As a result, KVM_RUN returns after enabling the timer, but before > incrementing 'userspace_irqchip_in_use': > kvm_arch_vcpu_run_pid_change() > ret = kvm_arm_pmu_v3_enable() > if (!vcpu->arch.pmu.created) > return -EINVAL; > if (ret) > return ret; > [...] > if (!irqchip_in_kernel(kvm)) > static_branch_inc(&userspace_irqchip_in_use); > - Userspace ignores the error and issues KVM_ARM_VCPU_INIT again. > Since the timer is already enabled, control moves through the > following flow, ultimately hitting the WARN_ON(): > kvm_timer_vcpu_reset() > if (timer->enabled) > kvm_timer_update_irq() > if (!userspace_irqchip()) > ret = kvm_vgic_inject_irq() > ret = vgic_lazy_init() > if (unlikely(!vgic_initialized(kvm))) > if (kvm->arch.vgic.vgic_model != > KVM_DEV_TYPE_ARM_VGIC_V2) > return -EBUSY; > WARN_ON(ret); > > Theoretically, since userspace_irqchip_in_use's functionality can be nit: this isn't theoretical at all. > simply replaced by '!irqchip_in_kernel()', get rid of the static key > to avoid the mismanagement, which also helps with the syzbot issue. Did you have a chance to check whether this had any negative impact on actual workloads? Since the entry/exit code is a bit of a hot spot, I'd like to make sure we're not penalising the common case (I only wrote this patch while waiting in an airport, and didn't test it at all). Any such data about it would be very welcome in the commit message. Thanks, M. -- Without deviation from the norm, progress is not possible.