From: Thiago Rafael Becker <trbecker@gmail.com>
To: linux-cifs@vger.kernel.org
Cc: Steve French <sfrench@samba.org>,
LKML <linux-kernel@vger.kernel.org>,
Thiago Rafael Becker <trbecker@gmail.com>
Subject: [PATCH] cifs: retry lookup and readdir when EAGAIN is returned.
Date: Tue, 15 Jun 2021 13:42:56 -0300 [thread overview]
Message-ID: <20210615164256.173715-1-trbecker@gmail.com> (raw)
According to the investigation performed by Jacob Shivers at Red Hat,
cifs_lookup and cifs_readdir leak EAGAIN when the user session is
deleted on the server. Fix this issue by implementing a retry with
limits, as is implemented in cifs_revalidate_dentry_attr.
Reproducer based on the work by Jacob Shivers:
~~~
$ cat readdir-cifs-test.sh
#!/bin/bash
# Install and configure powershell and sshd on the windows
# server as descibed in
# https://docs.microsoft.com/en-us/windows-server/administration/openssh/openssh_overview
# This script uses expect(1)
USER=dude
SERVER=192.168.0.2
RPATH=root
PASS='password'
function debug_funcs {
for line in $@ ; do
echo "func $line +p" > /sys/kernel/debug/dynamic_debug/control
done
}
function setup {
echo 1 > /proc/fs/cifs/cifsFYI
debug_funcs wait_for_compound_request \
smb2_query_dir_first cifs_readdir \
compound_send_recv cifs_reconnect_tcon \
generic_ip_connect cifs_reconnect \
smb2_reconnect_server smb2_reconnect \
cifs_readv_from_socket cifs_readv_receive
tcpdump -i eth0 -w cifs.pcap host 192.168.2.182 & sleep 5
dmesg -C
}
function test_call {
if [[ $1 == 1 ]] ; then
tracer="strace -tt -f -s 4096 -o trace-$(date -Iseconds).txt"
fi
# Change the command here to anything apropriate
$tracer ls $2 > /dev/null
res=$?
if [[ $1 == 1 ]] ; then
if [[ $res == 0 ]] ; then
1>&2 echo success
else
1>&2 echo "failure ($res)"
fi
fi
}
mountpoint /mnt > /dev/null || mount -t cifs -o username=$USER,pass=$PASS //$SERVER/$RPATH /mnt
test_call 0 /mnt/
/usr/bin/expect << EOF
set timeout 60
spawn ssh $USER@$SERVER
expect "yes/no" {
send "yes\r"
expect "*?assword" { send "$PASS\r" }
} "*?assword" { send "$PASS\r" }
expect ">" { send "powershell close-smbsession -force\r" }
expect ">" { send "exit\r" }
expect eof
EOF
sysctl -w vm.drop_caches=2 > /dev/null
sysctl -w vm.drop_caches=2 > /dev/null
setup
test_call 1 /mnt/
~~~
Signed-off-by: Thiago Rafael Becker <trbecker@gmail.com>
---
fs/cifs/dir.c | 4 ++++
fs/cifs/smb2ops.c | 5 +++++
2 files changed, 9 insertions(+)
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 6bcd3e8f7cda..7c641f9a3dac 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -630,6 +630,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
struct inode *newInode = NULL;
const char *full_path;
void *page;
+ int retry_count = 0;
xid = get_xid();
@@ -673,6 +674,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
cifs_dbg(FYI, "Full path: %s inode = 0x%p\n",
full_path, d_inode(direntry));
+again:
if (pTcon->posix_extensions)
rc = smb311_posix_get_inode_info(&newInode, full_path, parent_dir_inode->i_sb, xid);
else if (pTcon->unix_ext) {
@@ -687,6 +689,8 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
/* since paths are not looked up by component - the parent
directories are presumed to be good here */
renew_parental_timestamps(direntry);
+ } else if (rc == -EAGAIN && retry_count++ < 10) {
+ goto again;
} else if (rc == -ENOENT) {
cifs_set_time(direntry, jiffies);
newInode = NULL;
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 21ef51d338e0..d241e6af8fe4 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2325,6 +2325,7 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
struct smb2_query_directory_rsp *qd_rsp = NULL;
struct smb2_create_rsp *op_rsp = NULL;
struct TCP_Server_Info *server = cifs_pick_channel(tcon->ses);
+ int retry_count = 0;
utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
if (!utf16_path)
@@ -2372,10 +2373,14 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
smb2_set_related(&rqst[1]);
+again:
rc = compound_send_recv(xid, tcon->ses, server,
flags, 2, rqst,
resp_buftype, rsp_iov);
+ if (rc == -EAGAIN && retry_count++ < 10)
+ goto again;
+
/* If the open failed there is nothing to do */
op_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
if (op_rsp == NULL || op_rsp->sync_hdr.Status != STATUS_SUCCESS) {
--
2.31.1
next reply other threads:[~2021-06-15 16:44 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-15 16:42 Thiago Rafael Becker [this message]
2021-06-16 6:09 ` [PATCH] cifs: retry lookup and readdir when EAGAIN is returned ronnie sahlberg
2021-06-16 10:07 ` Aurélien Aptel
2021-06-16 15:40 ` Thiago Rafael Becker
2021-06-16 17:09 ` 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=20210615164256.173715-1-trbecker@gmail.com \
--to=trbecker@gmail.com \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sfrench@samba.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.