From patchwork Sat Apr 2 17:40:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Upton X-Patchwork-Id: 12799390 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 3A4BAC433EF for ; Sat, 2 Apr 2022 17:42:21 +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=+slMrmg0QA3Kmoyjh86JlDsH3ku/o51z8roMVgg34HI=; b=rQ6 7xR21HRdOUNUnQn8pX5mtnGbfQDVOa/Wb1c9J2+YSUFqc3FNO7n8s1+sAOWpkt7D2iqkw0Ibv3sST Br60dHGh/lAEGDhlsXqbAu7Bkgpu8OiYU6uYWo0ZiCNbtpYHk5/BvUuZ4OGfUxHOtOokruBquTz35 5kLphR5p3VXly4svFqr+KWmNzxkLy6/PctyE1dMFL/GUzRd35kkcA53JGYUG+hKxVNY7GXCeVVE0a 44nP8OQxFyEhK05bPGPoAeHTOepR95jchCjqonznTpIMOVrIxRqr55dQ9CQ/V2gHlgU6cuxcoteB4 e0eDn3FEzX9NUhH4BWAgr0MWBzBoGRA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nahk6-009jaX-8X; Sat, 02 Apr 2022 17:40:54 +0000 Received: from mail-il1-x149.google.com ([2607:f8b0:4864:20::149]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nahk3-009jZB-8x for linux-arm-kernel@lists.infradead.org; Sat, 02 Apr 2022 17:40:52 +0000 Received: by mail-il1-x149.google.com with SMTP id v15-20020a056e0213cf00b002c9f78d2e9bso3786208ilj.0 for ; Sat, 02 Apr 2022 10:40:49 -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=RL+g93Fijo1SZ8lwfn+Xp7o4bffPLL5ZnXUT6dIwBzU=; b=jczsRYruEZAo5WKmoWBK5h9QZfLDn8n1TXFMm6z7O5w5ywolfcnyEixifD0NojlNnV KAHqCd/80lYyWIxinRrWpQ6WI+9qcgcD/wT3SQMYw1TPYNWDO3yxvq5J/KUV+5zfUFHn N39L/g8DKNJXDMxWHTc+MtWTYW78lMwhKUfqG2kbIxFFEpvIZTO5xLeT4YHvil+DTTee nkg/53P1GB18loNhh6P4ztw61Rsa+JbDMPj+g0zX9HZ1CCi8nYLa29eQlnjUiXyyXu2D v6t/VdoaGhi/g9jKMdnpb1eNlcGnA8E4iU4S/Ti2iqU3vH90pLUfLeiG7SyeESxPYDQE oTxQ== 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=RL+g93Fijo1SZ8lwfn+Xp7o4bffPLL5ZnXUT6dIwBzU=; b=scDFM85Ram8jry//joD6dysnIkdk72b14sVYX/R7I6WCMmW8To/hIg/Nlb8u+X89mV B+emegbXqPGaxkO2DqGB2UlJfAOAF1UReb3b4awIhiNdo3aV33R354+xl7SDqImnBDSR e71aLoDyT0jGjLFP96H0Se6VOM0vUoXb27R27DnFgMf+21d4tQDtlyw45D7ymfMXmlN5 M+oeSN1lVH/8L8w7+gEmRAguY+HC1xBl8Z1qrUc0+9Mhw0IXj5ZM6/n7aPfQ58Y3DNIR rG2zJAzX7zm67HahPAYXfrTbUtx0VHy7Q0y/drnYh7lyytOz6CgKIr/T/cgrRXQulnRJ T+6A== X-Gm-Message-State: AOAM533HTsIKu+EFEc9x3rwkrjm+nV2VoeZUt7ygjtkS3MfdOhYwyM+N PQGS8MUk2wHRpvb0mHrMy2v7z7YzOf0= X-Google-Smtp-Source: ABdhPJzYn6lVkuhXlh2hTOvp36IECh6AGHkLwh7F6GtH58BHozTY1KxCiZKAVNa+Y+rlLwHIPZK/zxqBLAA= X-Received: from oupton.c.googlers.com ([fda3:e722:ac3:cc00:2b:ff92:c0a8:404]) (user=oupton job=sendgmr) by 2002:a05:6602:1645:b0:646:4780:fc28 with SMTP id y5-20020a056602164500b006464780fc28mr2144945iow.194.1648921248546; Sat, 02 Apr 2022 10:40:48 -0700 (PDT) Date: Sat, 2 Apr 2022 17:40:40 +0000 Message-Id: <20220402174044.2263418-1-oupton@google.com> Mime-Version: 1.0 X-Mailer: git-send-email 2.35.1.1094.g7c7d902a7c-goog Subject: [PATCH 0/4] KVM: arm64: 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-20220402_104051_396257_CEBF6DE6 X-CRM114-Status: GOOD ( 16.46 ) 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 takes care of the immediate problem by refusing to create the file if the VM directory does not exist. Patch 2 tones down logging around debugfs collisions. As demonstrated by the selftest, this is most likely to happen for a userspace bug, not KVM. The last two patches ensure the GIC FD actually gets closed by the selftests that use it. Patch 3 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 4 gets the arch_timer test to follow the well-behaved pattern. Applies cleanly to the first KVM pull (tagged kvm-5.18-1), at commit: c9b8fecddb5b ("KVM: use kvcalloc for array allocations") The series is intentionally *not* based on the kvmarm/fixes tree because there is a small dependency on commit: 456f89e0928a ("KVM: selftests: aarch64: Skip tests if we can't create a vgic-v3") which isn't present in the fixes branch. Tested on an Ampere Altra system in the following combinations: - Bad kernel + fixed selftests - Fixed kernel + bad selftests In both cases there was no dmesg spew and no unintended vgic-state file at the root of debugfs. Oliver Upton (4): KVM: arm64: vgic: Don't assume the VM debugfs directory exists KVM: Only log about debugfs directory collision once selftests: KVM: Don't leak GIC FD across dirty log test iterations selftests: KVM: Free the GIC FD when cleaning up in arch_timer arch/arm64/kvm/vgic/vgic-debug.c | 3 ++ .../selftests/kvm/aarch64/arch_timer.c | 15 +++++--- .../selftests/kvm/dirty_log_perf_test.c | 34 +++++++++++++++++-- virt/kvm/kvm_main.c | 2 +- 4 files changed, 46 insertions(+), 8 deletions(-)