From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:58142 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726543AbeJCVHJ (ORCPT ); Wed, 3 Oct 2018 17:07:09 -0400 From: Benjamin Coddington To: Alexander Viro Cc: linux-fsdevel@vger.kernel.org Subject: [PATCH v2] mnt: fix __detach_mounts infinite loop Date: Wed, 3 Oct 2018 10:18:33 -0400 Message-Id: <9d2d5b14bac03eb3cd9d6daa476a294e83a69ab6.1538576163.git.bcodding@redhat.com> In-Reply-To: <379eb36e37e77e3c71b2f6579664c525bda03f61.1538575707.git.bcodding@redhat.com> References: <379eb36e37e77e3c71b2f6579664c525bda03f61.1538575707.git.bcodding@redhat.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: v2: less whitespace 8<------------------------------------------------------------------------ Since commit ff17fa561a04 ("d_invalidate(): unhash immediately") immediately unhashes the dentry, we'll never return the mountpoint in lookup_mountpoint(), which can lead to an unbreakable loop in d_invalidate(). I have reports of NFS clients getting into this condition after the server removes an export of an existing mount created through follow_automount(), but I suspect there are various other ways to produce this problem if we hunt down users of d_invalidate(). For example, it is possible to get into this state by using XFS' d_invalidate() call in xfs_vn_unlink(): truncate -s 100m img{1,2} mkfs.xfs -q -n version=ci img1 mkfs.xfs -q -n version=ci img2 mkdir -p /mnt/xfs mount img1 /mnt/xfs mkdir /mnt/xfs/sub1 mount img2 /mnt/xfs/sub1 cat > /mnt/xfs/sub1/foo & umount -l /mnt/xfs/sub1 mount img2 /mnt/xfs/sub1 mount --make-private /mnt/xfs mkdir /mnt/xfs/sub2 mount --move /mnt/xfs/sub1 /mnt/xfs/sub2 rmdir /mnt/xfs/sub1 Fix this by moving the check for an unlinked dentry out of the detach_mounts() path. Fixes: ff17fa561a04 ("d_invalidate(): unhash immediately") Signed-off-by: Benjamin Coddington --- fs/namespace.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index bd2f4c68506a..ad8feb9371f2 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -780,9 +780,6 @@ static struct mountpoint *lookup_mountpoint(struct dentry *dentry) hlist_for_each_entry(mp, chain, m_hash) { if (mp->m_dentry == dentry) { - /* might be worth a WARN_ON() */ - if (d_unlinked(dentry)) - return ERR_PTR(-ENOENT); mp->m_count++; return mp; } @@ -796,6 +793,9 @@ static struct mountpoint *get_mountpoint(struct dentry *dentry) int ret; if (d_mountpoint(dentry)) { + /* might be worth a WARN_ON() */ + if (d_unlinked(dentry)) + return ERR_PTR(-ENOENT); mountpoint: read_seqlock_excl(&mount_lock); mp = lookup_mountpoint(dentry); -- 2.14.3