From patchwork Wed Apr 6 23:56:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Upton X-Patchwork-Id: 12804256 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 33460C433F5 for ; Wed, 6 Apr 2022 23:57:50 +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:Mime-Version: Message-Id:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To: References:List-Owner; bh=GscXKANj5zDfXwMdMyxmjHad+YzKiTv6E5rquVN8NaY=; b=A25 XWbWQSFoBYJmxv0+IiyQha4QiVxu9onKlo+u1p5wMYz62TPoKMzDZ5gTCvzSBPDwDUSgzvdhA8b01 aMXTPhLSDEgyyPR2TH29TMlnu6xvwwCucnqU6XmVT0XV1LtUvYi02JB5WK5IwggG1ozif36Cb6IN5 vtw1sqaPPqVnI9CjVeOmqpo4XByg3seL2TLfibg51ITvqu9Mp9p5Y7doFfFs7cmgFzQk7A7ZwA0wz 4p/ShIc82fJLlmQYEL43CQ1LmBn7eGoi8y0K9Qd338xy0GU4IbI1NPE5WhEwLfmwSJbnQXC0b0pYD ZGqeYqvfI1wCiJith53gcu6BE/mzBCg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ncFVi-008L1d-Bg; Wed, 06 Apr 2022 23:56:26 +0000 Received: from mail-ot1-x349.google.com ([2607:f8b0:4864:20::349]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ncFVe-008L0z-Vq for linux-arm-kernel@lists.infradead.org; Wed, 06 Apr 2022 23:56:24 +0000 Received: by mail-ot1-x349.google.com with SMTP id 92-20020a9d0de5000000b005b2731fb946so2017103ots.4 for ; Wed, 06 Apr 2022 16:56:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:message-id:mime-version:subject:from:to:cc; bh=fwRsymA/7INPtz5uOlj06FEv7kVaB+utCnxy48AEBxo=; b=tHLPte6eqKvlyXBDRIPYKCLDrChMHfeEZx8L7EPrTciBSzA6GjKpM2hUwRIYgHL8Vf WbCFZFSQ9yjysfEoEe4joScrVi5mWuEF2Vob3FNBUpKhJeBqSQ5OUv0iGoQdYiOK3Fu4 wpNOytV1B1GSyY1bwSDaky8abQgztkae3aVbObEoU+tWdOgD8KIv3x9c/q1kpnV9aZIS 22nklc+Qm6pR7fNYrGUQ8dIFklA+X3FGgUoHCSvSGaoFRvqAVXXDZ7VJpPdOtnigUFx1 88GQMckg6WyOIQp4YdnS0X6Y4sXKV8n49HaaujyeVOacYu/K7GGyZce1+22r1+vUdDZf MTbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:message-id:mime-version:subject:from:to:cc; bh=fwRsymA/7INPtz5uOlj06FEv7kVaB+utCnxy48AEBxo=; b=DTtG8lr/1OBkN2g+Yb+XczPCDGMgzNK8NWV4Wj85rt43SWFuyM/dEmKTRxS7AtWqvr aY4c224jrfi7Z5lqsworHlzbtjAE2sRTOkppkkRbvA3lao5H+FBjH1P7sTr4qtsypDlq 8nLbco3MM4dRZeKS44FpnvtO8CzYu0YaXO5RPHlGDsem0Q3P6Dtv3pIatI32M6N3ut7S RsGSt9eB1iwe5/MdjgQqb6y1vS8PktknPcxCO2KF45S7MmeduF1nmP1Vtn6VKjg3S/mw iiEjtitxwlfUl6LU11OgE5QNy0WeiEXWAbZMUaau8oG2Q650d58POoPdo3nEu9XZiRt0 8W6w== X-Gm-Message-State: AOAM531Z87mYPK/latfYbIPr7YW3+nLJhG6PV02zS1zF9V6EmEU6d1NM u57+cAivZYxvnhhimn27cZJFhaCovos= X-Google-Smtp-Source: ABdhPJzj/VGxAMS1iBtHTXCMn1pekmLm2UMBGpiITgnFbH8BraR4aqQ8P5nQ2xVOA8GyeF2fPqDC5v3aSnw= X-Received: from oupton.c.googlers.com ([fda3:e722:ac3:cc00:2b:ff92:c0a8:404]) (user=oupton job=sendgmr) by 2002:a05:6870:c698:b0:de:8a16:c37 with SMTP id cv24-20020a056870c69800b000de8a160c37mr5259350oab.191.1649289380523; Wed, 06 Apr 2022 16:56:20 -0700 (PDT) Date: Wed, 6 Apr 2022 23:56:12 +0000 Message-Id: <20220406235615.1447180-1-oupton@google.com> Mime-Version: 1.0 X-Mailer: git-send-email 2.35.1.1094.g7c7d902a7c-goog Subject: [PATCH v3 0/3] KVM: Fix use-after-free in debugfs From: Oliver Upton To: kvmarm@lists.cs.columbia.edu Cc: kvm@vger.kernel.org, Marc Zyngier , James Morse , Alexandru Elisei , Suzuki K Poulose , linux-arm-kernel@lists.infradead.org, Peter Shier , Ricardo Koller , Reiji Watanabe , Paolo Bonzini , Sean Christopherson , Oliver Upton X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220406_165623_064055_CCC828DA X-CRM114-Status: GOOD ( 15.11 ) 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 Funny enough, dirty_log_perf_test on arm64 highlights some issues around the use of debugfs in KVM. The test leaks a GIC FD across test iterations, and as such the associated VM is never destroyed. Nonetheless, the VM FD is reused for the next VM, which collides with the old debugfs directory. Where things get off is when the vgic-state debugfs file is created. KVM does not check if the VM directory exists before creating the file, which results in the file being added to the root of debugfs when the aforementioned collision occurs. Since KVM relies on deleting the VM directory to clean up all associated files, the errant vgic-state file never gets cleaned up. Poking the file after the VM is destroyed is a use-after-free :) Patch 1 shuts the door on any mistaken debugfs file creations by initializing kvm->debugfs_dentry with ERR_PTR() instead of NULL. The last two patches ensure the GIC FD actually gets closed by the selftests that use it. Patch 2 is a genuine bug fix since it will create multiple VMs for a single test run. The arch_timer test also happens to leak the GIC FD, though it is benign since the test creates a single VM. Patch 3 gets the arch_timer test to follow the well-behaved pattern. Applies cleanly to 5.18-rc1. Tested on an Ampere Altra system in the following combinations: - Bad kernel + fixed selftests - Fixed kernel + bad selftests In both cases there was no vgic-state file at the root of debugfs. Additionally, I made sure the VM debugfs directory was in fact cleaned up at exit. v1: https://lore.kernel.org/kvm/20220402174044.2263418-1-oupton@google.com/ v2: https://lore.kernel.org/kvm/20220404182119.3561025-1-oupton@google.com/ v2 -> v3: - Fix error checking in kvm_destroy_vm_debugfs(). Embarrassingly, v2 worsened the bug by not cleaning up the VM directory... (Marc) v1 -> v2: - Initialize kvm->debugfs_dentry to ERR_PTR(-ENOENT) to prevent the creation of all VM debug files, not just vgic-state. - Leave logging as-is for now. Consider dropping altogether in a later patch (Sean) - Collect R-b from Jing Oliver Upton (3): KVM: Don't create VM debugfs files outside of the VM directory selftests: KVM: Don't leak GIC FD across dirty log test iterations selftests: KVM: Free the GIC FD when cleaning up in arch_timer .../selftests/kvm/aarch64/arch_timer.c | 15 +++++--- .../selftests/kvm/dirty_log_perf_test.c | 34 +++++++++++++++++-- virt/kvm/kvm_main.c | 10 ++++-- 3 files changed, 50 insertions(+), 9 deletions(-)