From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:31810 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727416AbeJVWdi (ORCPT ); Mon, 22 Oct 2018 18:33:38 -0400 From: "Benjamin Coddington" To: "Alexander Viro" Cc: linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v2] mnt: fix __detach_mounts infinite loop Date: Mon, 22 Oct 2018 10:14:53 -0400 Message-ID: <477E0417-EF75-4A2D-AB3E-0C5E6B8E1D67@redhat.com> In-Reply-To: <9d2d5b14bac03eb3cd9d6daa476a294e83a69ab6.1538576163.git.bcodding@redhat.com> References: <379eb36e37e77e3c71b2f6579664c525bda03f61.1538575707.git.bcodding@redhat.com> <9d2d5b14bac03eb3cd9d6daa476a294e83a69ab6.1538576163.git.bcodding@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; format=flowed Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Al, would you be willing to take a look at this one? Ben On 3 Oct 2018, at 10:18, Benjamin Coddington wrote: > 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