linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Aurélien Aptel" <aaptel@suse.com>
To: linux-cifs@vger.kernel.org, linux-fsdevel@vger.kernel.org
Cc: smfrench@gmail.com, Aurelien Aptel <aaptel@suse.com>,
	Paulo Alcantara <pc@cjr.nz>
Subject: [PATCH v1] cifs: make nested cifs mount point dentries always valid to deal with signaled 'df'
Date: Fri, 29 Jan 2021 18:13:16 +0100	[thread overview]
Message-ID: <20210129171316.13160-1-aaptel@suse.com> (raw)

From: Aurelien Aptel <aaptel@suse.com>

Assuming
- //HOST/a is mounted on /mnt
- //HOST/b is mounted on /mnt/b

On a slow connection, running 'df' and killing it while it's
processing /mnt/b can make cifs_get_inode_info() returns -ERESTARTSYS.

This triggers the following chain of events:
=> the dentry revalidation fail
=> dentry is put and released
=> superblock associated with the dentry is put
=> /mnt/b is unmounted

This quick fix makes cifs_d_revalidate() always succeed (mark as
valid) on cifs dentries which are also mount points.

Signed-off-by: Aurelien Aptel <aaptel@suse.com>
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---

I have managed to reproduce this bug with the following script.  It
uses tc with netem discipline (CONFIG_NET_SCH_NETEM=y) to simulate
network delays.

#!/bin/bash

#
# reproducing bsc#1177440
#
# nested mount point gets unmounted when process is signaled
#
set -e

share1=//192.168.2.110/scratch
share2=//192.168.2.110/test
opts="username=administrator,password=aaptel-42,vers=1.0,actimeo=0"

cleanup() {
    # reset delay
    tc qdisc del dev eth0 root
    mount|grep -q /mnt/nest/a && umount /mnt/nest/a
    mount|grep -q /mnt/nest && umount /mnt/nest

    echo 'module cifs -p' > /sys/kernel/debug/dynamic_debug/control
    echo 'file fs/cifs/* -p' > /sys/kernel/debug/dynamic_debug/control
    echo 0 > /proc/fs/cifs/cifsFYI
    echo 0 > /sys/module/dns_resolver/parameters/debug
    echo 1 > /proc/sys/kernel/printk_ratelimit
    
}

trap cleanup EXIT

nbcifs() {
    mount|grep cifs|wc -l
}

reset() {
    echo "unmounting and reloading cifs.ko"
    mount|grep -q /mnt/nest/a && umount /mnt/nest/a
    mount|grep -q /mnt/nest && umount /mnt/nest
    sleep 1
    lsmod|grep -q cifs && ( modprobe -r cifs &> /dev/null || true )
    lsmod|grep -q cifs || ( modprobe cifs &> /dev/null  || true )
}

mnt() {
    dmesg --clear
    echo 'module cifs +p' > /sys/kernel/debug/dynamic_debug/control
    echo 'file fs/cifs/* +p' > /sys/kernel/debug/dynamic_debug/control
    echo 1 > /proc/fs/cifs/cifsFYI
    echo 1 > /sys/module/dns_resolver/parameters/debug
    echo 0 > /proc/sys/kernel/printk_ratelimit

    echo "mounting"
    mkdir -p /mnt/nest
    mount.cifs $share1 /mnt/nest -o "$opts"
    mkdir -p /mnt/nest/a
    mount.cifs $share2 /mnt/nest/a -o "$opts"
}

# add fake delay
tc qdisc add dev eth0 root netem delay 300ms

while :; do
    reset
    mnt
    n=$(nbcifs)    
    echo "starting df with $n mounts"
    df & 
    pid=$!
    sleep 1.5
    kill $pid || true
    x=$(nbcifs)
    echo "stopping with $x mounts"
    if [ $x -lt $n ]; then
        echo "lost mounts"
        dmesg > kernel.log
        exit 1
    fi
done



fs/cifs/dir.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 68900f1629bff..876ef01628538 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -741,6 +741,10 @@ cifs_d_revalidate(struct dentry *direntry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	/* nested cifs mount point are always valid */
+	if (d_mountpoint(direntry))
+		return 1;
+
 	if (d_really_is_positive(direntry)) {
 		inode = d_inode(direntry);
 		if ((flags & LOOKUP_REVAL) && !CIFS_CACHE_READ(CIFS_I(inode)))
-- 
2.29.2


             reply	other threads:[~2021-01-29 17:16 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-29 17:13 Aurélien Aptel [this message]
2021-01-31  9:28 ` [PATCH v1] cifs: make nested cifs mount point dentries always valid to deal with signaled 'df' Shyam Prasad N
2021-02-01 10:31   ` Aurélien Aptel
2021-02-01 16:51     ` Shyam Prasad N
2021-02-02 11:00       ` Aurélien Aptel
2021-02-02 11:16       ` [PATCH v2] cifs: report error instead of invalid when revalidating a dentry fails Aurélien Aptel
2021-02-02 17:09         ` Shyam Prasad N
2021-02-02 17:34           ` Aurélien Aptel
2021-02-02 17:42           ` [PATCH v3] " Aurélien Aptel
2021-02-02 18:26             ` Shyam Prasad N
2021-02-02 18:34               ` Aurélien Aptel
2021-02-03  4:24                 ` Shyam Prasad N
2021-02-05 13:32                 ` Shyam Prasad N
2021-02-05 14:42                   ` [PATCH v4] " Aurélien Aptel
2021-02-05 14:52                     ` Shyam Prasad N
2021-02-05 19:22                       ` Steve French
2021-02-05 22:31                     ` Steve French
2021-02-03  4:11             ` [PATCH v3] " Steve French

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210129171316.13160-1-aaptel@suse.com \
    --to=aaptel@suse.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=pc@cjr.nz \
    --cc=smfrench@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).