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 X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B66B5C38A30 for ; Sun, 19 Apr 2020 19:45:43 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 586DE2145D for ; Sun, 19 Apr 2020 19:45:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 586DE2145D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 80E368E0005; Sun, 19 Apr 2020 15:45:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7BF1A8E0003; Sun, 19 Apr 2020 15:45:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6D5548E0005; Sun, 19 Apr 2020 15:45:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0227.hostedemail.com [216.40.44.227]) by kanga.kvack.org (Postfix) with ESMTP id 55C4F8E0003 for ; Sun, 19 Apr 2020 15:45:42 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 0832F180ACEE4 for ; Sun, 19 Apr 2020 19:45:42 +0000 (UTC) X-FDA: 76725634524.07.cart26_4ed114cdfad07 X-HE-Tag: cart26_4ed114cdfad07 X-Filterd-Recvd-Size: 7506 Received: from mail-pl1-f194.google.com (mail-pl1-f194.google.com [209.85.214.194]) by imf01.hostedemail.com (Postfix) with ESMTP for ; Sun, 19 Apr 2020 19:45:41 +0000 (UTC) Received: by mail-pl1-f194.google.com with SMTP id k18so3128417pll.6 for ; Sun, 19 Apr 2020 12:45:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=4nBDa0rXFg9iHiCCMSKdC08ZVJ5XJ1HD+NLLqlL75QE=; b=sCcyQLxlHHjuh8Zpfz9HaNen+ThbpMhAgf/1QwDXWb1HBoJC9vo0wfon55ZrSCXHBF 0WycFKGRtsiHNh/q5SNfrNSXebBpXv7OC7UDh2eYowPaXxsbYux3G4pjMTRwlYgLxGj7 qSHcbG+mPf1fI76Sr/TEXJv2mv59pm9aIhrlfcLszS2XDjuUkTYpf+JC9eMEm5Zet8bd cO3ckP1mlaLO1YJ19rmYuSh+J7yAmkn/W6rLOCOLK4lBDdiRCEQ0OpnDUptQrHzzgbsU 1BOVz0PHJU2gH9GdS1gmX51M+c7zWE1fgeI1FAIQrwlc39DtBnaADb2IKM4kM8d73Q5a B/cQ== X-Gm-Message-State: AGi0PuYoRjiUomauBa+Up79C/XcaEXdGt8pRT+Kn01MGkfFzxEb+NAO1 OGC9s8DRjdDOyN2tyQEjxIM= X-Google-Smtp-Source: APiQypJayFgKefqgg6H/JR67sswmZGgea+8/ivT+/Vf5pPT068xt+HAMYcOsqSUtrFH/BBGo5pJ52w== X-Received: by 2002:a17:90a:9202:: with SMTP id m2mr8028414pjo.109.1587325540421; Sun, 19 Apr 2020 12:45:40 -0700 (PDT) Received: from 42.do-not-panic.com (42.do-not-panic.com. [157.230.128.187]) by smtp.gmail.com with ESMTPSA id i8sm22879992pgr.82.2020.04.19.12.45.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 19 Apr 2020 12:45:39 -0700 (PDT) Received: by 42.do-not-panic.com (Postfix, from userid 1000) id 7E737403EA; Sun, 19 Apr 2020 19:45:38 +0000 (UTC) From: Luis Chamberlain To: axboe@kernel.dk, viro@zeniv.linux.org.uk, bvanassche@acm.org, gregkh@linuxfoundation.org, rostedt@goodmis.org, mingo@redhat.com, jack@suse.cz, ming.lei@redhat.com, nstange@suse.de, akpm@linux-foundation.org Cc: mhocko@suse.com, yukuai3@huawei.com, linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Luis Chamberlain Subject: [PATCH v2 00/10] block: fix blktrace debugfs use after free Date: Sun, 19 Apr 2020 19:45:19 +0000 Message-Id: <20200419194529.4872-1-mcgrof@kernel.org> X-Mailer: git-send-email 2.23.0.rc1 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Upstream kernel.org korg#205713 [0] states that there is a UAF in the core debugfs debugfs_remove() function, and has gone through pushing for a CVE for this, CVE-2019-19770 [1]. This patch series disputes that CVE, and shows how the issue was just a complex misuse of debugfs within blktrace and fixes it. On this v2 I've dropped two patches from my last series which are not needed to ensure we can move back to a synchronous request_queue removal. I've also addressed Ming's feedback on ensuring we keep functionality working for when paritions are used for a blktrace. That effort lead me to ensuring we don't try to overwrite the request_queue debugfs_dir, and add sanity checks in place so that what we give back only what is expected. Although my v1 patches also had fixed the kernel splat we get when we try to reproduce the issue: debugfs: Directory 'loop0' with parent 'block' already present! This v2 series now provides a clear explanation for *why* this was ultimately one of the reasons why we ended up with a crash. The commit log for the actual fix, patch 3/10, "blktrace: fix debugfs use after free" has also been extended to provide a better explanation as to *how* overwriting the debugfs_dir leads to an eventual panic with blktrace. I hope that helps, as it seems the root cause was still not well explained in the commit log. To make review easier, I've also added some helper functions with no functional changes at first, and only extended them later. Also changed is blk_queue_debugfs_register() to return an int, we do this to not make the fact that we don't check for errors on register_disk() or add_disk() any worse. After the patch 3/10 "blktrace: fix debugfs use after free", is applied, with the pr_warns(), and prior to reverting back to synchronous request_q= ueue removal, if we try to reproduce the issue with break-blktrace [2]'s ./run_0001.sh script, we'd see:: blk_debugfs: loop0 : registering request_queue debugfs directory twice is= not allowed blktrace: loop0: request_queue parent is gone And sometimes only: blk_debugfs: loop0 : registering request_queue debugfs directory twice is= not allowed After we revert back to synchronous request_queue removal this should no longer be possible, and if it is, we want to hear about it. To help with this two patches are added which change pr_warn() to BUG_ON()s after we f= lip back to synchronous request_queue removal. Note that on patch 6/10 "blk-debugfs: upgrade warns to BUG_ON() if directory" I explain the syfs layout between a gendisk and the request_queue. By reverting back to synchronous request_queue removal, if someone manages to figure out a way to create a clash with registering block devices, we expect to see a sysfs clash now instead of a clash with debugfs, as the debugfs directory is removed now always first, prior to clearing out the sysfs dir. *If* there are races possible in these areas, we want to hear about them, and the BUG_ON()s should make it clearer *where* the real issue is coming from. Having an asynchronous request_queue removal has exposed other bugs lingering around, however most importantly I think its revealing more the value of adding error handling for __device_add_disk() and friends. If its encouraged I could take a stab at finally addressing that for good. You can find this code on my git tree, on the 20200417-blktrace-fixes branch, which is based on linux-next tag next-20200417 [3]. [0] https://bugzilla.kernel.org/show_bug.cgi?id=3D205713 = =20 [1] https://nvd.nist.gov/vuln/detail/CVE-2019-19770=20 [2] https://github.com/mcgrof/break-blktrace [3] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git= /log/?h=3D20200417-blktrace-fixes Luis Chamberlain (10): block: move main block debugfs initialization to its own file blktrace: move blktrace debugfs creation to helper function blktrace: fix debugfs use after free block: revert back to synchronous request_queue removal blktrace: upgrade warns to BUG_ON() on unexpected circmunstances blk-debugfs: upgrade warns to BUG_ON() if directory is already found blktrace: move debugfs file creation to its own function blktrace: add checks for created debugfs files on setup block: panic if block debugfs dir is not created block: put_device() if device_add() fails block/Makefile | 1 + block/blk-core.c | 28 +++++--- block/blk-debugfs.c | 39 ++++++++++++ block/blk-mq-debugfs.c | 5 -- block/blk-sysfs.c | 47 ++++++++------ block/blk.h | 18 ++++++ block/genhd.c | 4 +- include/linux/blkdev.h | 7 +- include/linux/blktrace_api.h | 1 + kernel/trace/blktrace.c | 120 +++++++++++++++++++++++++++++++---- 10 files changed, 218 insertions(+), 52 deletions(-) create mode 100644 block/blk-debugfs.c --=20 2.25.1