From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from sub5.mail.dreamhost.com ([208.113.200.129]:37655 "EHLO homiemail-a124.g.dreamhost.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S937604AbdAFHAb (ORCPT ); Fri, 6 Jan 2017 02:00:31 -0500 Received: from homiemail-a124.g.dreamhost.com (localhost [127.0.0.1]) by homiemail-a124.g.dreamhost.com (Postfix) with ESMTP id B66C160000D03 for ; Thu, 5 Jan 2017 23:00:30 -0800 (PST) Received: from kmjvbox (c-73-70-90-212.hsd1.ca.comcast.net [73.70.90.212]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: kjlx@templeofstupid.com) by homiemail-a124.g.dreamhost.com (Postfix) with ESMTPSA id 9830F60001B0A for ; Thu, 5 Jan 2017 23:00:30 -0800 (PST) Date: Thu, 5 Jan 2017 23:00:29 -0800 From: Krister Johansen To: "Eric W. Biederman" Cc: Al Viro , linux-fsdevel@vger.kernel.org Subject: Re: [PATCH] mnt: Protect the mountpoint hashtable with mount_lock Message-ID: <20170106070029.GC2707@templeofstupid.com> References: <20161231041001.GA2448@templeofstupid.com> <20161231061729.GX1555@ZenIV.linux.org.uk> <874m1hdkyv.fsf@xmission.com> <20170103014806.GA1555@ZenIV.linux.org.uk> <87ful07ryd.fsf@xmission.com> <20170103040052.GB1555@ZenIV.linux.org.uk> <87y3yr32ig.fsf@xmission.com> <87shoz32g8.fsf_-_@xmission.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87shoz32g8.fsf_-_@xmission.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, Jan 04, 2017 at 04:53:59PM +1300, Eric W. Biederman wrote: > > Protecting the mountpoint hashtable with namespace_sem was sufficient > until a call to umount_mnt was added to mntput_no_expire. At which > point it became possible for multiple calls of put_mountpoint on > the same hash chain to happen on the same time. > > Kristen Johansen reported: > > This can cause a panic when simultaneous callers of put_mountpoint > > attempt to free the same mountpoint. This occurs because some callers > > hold the mount_hash_lock, while others hold the namespace lock. Some > > even hold both. > > > > In this submitter's case, the panic manifested itself as a GP fault in > > put_mountpoint() when it called hlist_del() and attempted to dereference > > a m_hash.pprev that had been poisioned by another thread. > > Al Viro observed that the simple fix is to switch from using the namespace_sem > to the mount_lock to protect the mountpoint hash table. > > I have taken Al's suggested patch moved put_mountpoint in pivot_root > (instead of taking mount_lock an additional time), and have replaced > new_mountpoint with get_mountpoint a function that does the hash table > lookup and addition under the mount_lock. The introduction of get_mounptoint > ensures that only the mount_lock is needed to manipulate the mountpoint > hashtable. > > d_set_mounted is modified to only set DCACHE_MOUNTED if it is not > already set. This allows get_mountpoint to use the setting of > DCACHE_MOUNTED to ensure adding a struct mountpoint for a dentry > happens exactly once. > > Cc: stable@vger.kernel.org > Fixes: ce07d891a089 ("mnt: Honor MNT_LOCKED when detaching mounts") > Reported-by: Krister Johansen > Suggested-by: Al Viro > Signed-off-by: "Eric W. Biederman" > --- Sorry for the slow reply. This looks right to me. I just pulled in the patch and went through all of the code paths in cscope. Everything is now under the mount_lock, which solves the problem from my perspective. Feel free to put me down as a reviewed-by if my vote counts.` There's another issue with MNT_LOCKED and detached mounts that I've been investigating. I'd be curious to get your opinion before I write any code. I'll send that out in a separate e-mail, though. -K