Received: by 2002:a05:6a10:1287:0:0:0:0 with SMTP id d7csp6127738pxv; Thu, 29 Jul 2021 07:10:43 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyvqU0/MKa2xjQvWobkQPBR5qWeNyzioDMK7CBzZAGmFMAsX1jtHPWfqrvuyKePqppW0wGM X-Received: by 2002:a05:6e02:1ca8:: with SMTP id x8mr3618176ill.259.1627567842875; Thu, 29 Jul 2021 07:10:42 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1627567842; cv=none; d=google.com; s=arc-20160816; b=cUO93Bk2TFSjJY3aX/IXNsZyQBHZxOnEhYQG/AzXM1PWnEzIbCUx2OuGp8RMFMUrlR z+ucuIWCDlJyc9VPGQpWpPv6J+7bayX40vzlRY8nItsuumfa6UmWLBvHIavaurI21owv B93COhUSsDXx+pHaC609JSR6EfGghRzLc6YPT0OkYX9xzwRjlABqi3c1PIw8WKSoM4Oh t1VZ/M1iSx9z6cL9Cm85Eyy8P72SHjLENMT3yGSFodqm5wZ8x4y65q0R6TRPP1M+XXzt NmigOW5EQbYAuf3p362LPfEoeqO9AZBdqlxwXxdWA4z3PDtk6NTrXQLgqd3fngTKO8Tw 9yoA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=VXkKTIIF/KGjvcnQg6Yn9XxLB7c21HX8fcEFvl0y0E8=; b=wS4wk+ep4MHN90koS10zbhlESp9P0l/cdGLEqYoiwDE6mPGtclrHvBVW5pfYetV+br cye40U/MmxTz3q4jwjURJIqUg4y1aBWCVqrmmzCfZ6WeqqXBBD8Xd8yBPtvx7W6dImWq W85lsQzpFO4Bxto8FR9AR/ulPtLyAEWsbAm4au1/FIrwxFRrqrDH+zPkX6/jS9n/iYG5 wgXh2cEt1r8pIhJxqrJpgkoaCrt1NXqWJ/YJkrbYZ6NJbd2XV4sf8Bm6ZVRU2+Yuu4Ku ojMtXR+S6UkFZvzNmFvK9p56FQkk1XESc/mP1n5w28YA56HwjEuILrb7p6tO2qlRzb7T Vsug== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=vSEh8ze7; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id s8si3842797ilo.76.2021.07.29.07.10.26; Thu, 29 Jul 2021 07:10:42 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=vSEh8ze7; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239230AbhG2OIa (ORCPT + 99 others); Thu, 29 Jul 2021 10:08:30 -0400 Received: from mail.kernel.org ([198.145.29.99]:49886 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238473AbhG2OA0 (ORCPT ); Thu, 29 Jul 2021 10:00:26 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 1361460EB2; Thu, 29 Jul 2021 13:59:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1627567170; bh=yHdO8hiRPBFyTHKu7W1+7ZN+QmOuhC4/z5e6gOaxctI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vSEh8ze7NjHBRapl4YmH+T63QaqGjatm9N4lTFh0JXMvu/4UbqMU+yhh4HFna/W5/ M9xYp0PWRTtvt3uO6Y05Zpc0CwPP7/KOtiuo6dTzjb8dOsDWeks9ddhqzbqBUhTdIJ BWE/OLPc986P2gCEzJ1pSvQ/dl9tp8YaxsmPzEho= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Al Viro , Tejun Heo , Zefan Li , Johannes Weiner , Richard Purdie , Paul Gortmaker Subject: [PATCH 5.13 03/22] cgroup1: fix leaked context root causing sporadic NULL deref in LTP Date: Thu, 29 Jul 2021 15:54:34 +0200 Message-Id: <20210729135137.441283314@linuxfoundation.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210729135137.336097792@linuxfoundation.org> References: <20210729135137.336097792@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Paul Gortmaker commit 1e7107c5ef44431bc1ebbd4c353f1d7c22e5f2ec upstream. Richard reported sporadic (roughly one in 10 or so) null dereferences and other strange behaviour for a set of automated LTP tests. Things like: BUG: kernel NULL pointer dereference, address: 0000000000000008 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 0 P4D 0 Oops: 0000 [#1] PREEMPT SMP PTI CPU: 0 PID: 1516 Comm: umount Not tainted 5.10.0-yocto-standard #1 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-48-gd9c812dda519-prebuilt.qemu.org 04/01/2014 RIP: 0010:kernfs_sop_show_path+0x1b/0x60 ...or these others: RIP: 0010:do_mkdirat+0x6a/0xf0 RIP: 0010:d_alloc_parallel+0x98/0x510 RIP: 0010:do_readlinkat+0x86/0x120 There were other less common instances of some kind of a general scribble but the common theme was mount and cgroup and a dubious dentry triggering the NULL dereference. I was only able to reproduce it under qemu by replicating Richard's setup as closely as possible - I never did get it to happen on bare metal, even while keeping everything else the same. In commit 71d883c37e8d ("cgroup_do_mount(): massage calling conventions") we see this as a part of the overall change: -------------- struct cgroup_subsys *ss; - struct dentry *dentry; [...] - dentry = cgroup_do_mount(&cgroup_fs_type, fc->sb_flags, root, - CGROUP_SUPER_MAGIC, ns); [...] - if (percpu_ref_is_dying(&root->cgrp.self.refcnt)) { - struct super_block *sb = dentry->d_sb; - dput(dentry); + ret = cgroup_do_mount(fc, CGROUP_SUPER_MAGIC, ns); + if (!ret && percpu_ref_is_dying(&root->cgrp.self.refcnt)) { + struct super_block *sb = fc->root->d_sb; + dput(fc->root); deactivate_locked_super(sb); msleep(10); return restart_syscall(); } -------------- In changing from the local "*dentry" variable to using fc->root, we now export/leave that dentry pointer in the file context after doing the dput() in the unlikely "is_dying" case. With LTP doing a crazy amount of back to back mount/unmount [testcases/bin/cgroup_regression_5_1.sh] the unlikely becomes slightly likely and then bad things happen. A fix would be to not leave the stale reference in fc->root as follows: --------------                 dput(fc->root); + fc->root = NULL;                 deactivate_locked_super(sb); -------------- ...but then we are just open-coding a duplicate of fc_drop_locked() so we simply use that instead. Cc: Al Viro Cc: Tejun Heo Cc: Zefan Li Cc: Johannes Weiner Cc: stable@vger.kernel.org # v5.1+ Reported-by: Richard Purdie Fixes: 71d883c37e8d ("cgroup_do_mount(): massage calling conventions") Signed-off-by: Paul Gortmaker Signed-off-by: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/internal.h | 1 - include/linux/fs_context.h | 1 + kernel/cgroup/cgroup-v1.c | 4 +--- 3 files changed, 2 insertions(+), 4 deletions(-) --- a/fs/internal.h +++ b/fs/internal.h @@ -61,7 +61,6 @@ extern void __init chrdev_init(void); */ extern const struct fs_context_operations legacy_fs_context_ops; extern int parse_monolithic_mount_data(struct fs_context *, void *); -extern void fc_drop_locked(struct fs_context *); extern void vfs_clean_context(struct fs_context *fc); extern int finish_clean_context(struct fs_context *fc); --- a/include/linux/fs_context.h +++ b/include/linux/fs_context.h @@ -139,6 +139,7 @@ extern int vfs_parse_fs_string(struct fs extern int generic_parse_monolithic(struct fs_context *fc, void *data); extern int vfs_get_tree(struct fs_context *fc); extern void put_fs_context(struct fs_context *fc); +extern void fc_drop_locked(struct fs_context *fc); /* * sget() wrappers to be called from the ->get_tree() op. --- a/kernel/cgroup/cgroup-v1.c +++ b/kernel/cgroup/cgroup-v1.c @@ -1225,9 +1225,7 @@ int cgroup1_get_tree(struct fs_context * ret = cgroup_do_get_tree(fc); if (!ret && percpu_ref_is_dying(&ctx->root->cgrp.self.refcnt)) { - struct super_block *sb = fc->root->d_sb; - dput(fc->root); - deactivate_locked_super(sb); + fc_drop_locked(fc); ret = 1; }