linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Thiago Rafael Becker <trbecker@gmail.com>,
	Ronnie Sahlberg <lsahlber@redhat.com>,
	Steve French <stfrench@microsoft.com>,
	Sasha Levin <sashal@kernel.org>,
	linux-cifs@vger.kernel.org, samba-technical@lists.samba.org
Subject: [PATCH AUTOSEL 5.13 40/59] cifs: retry lookup and readdir when EAGAIN is returned.
Date: Mon,  5 Jul 2021 11:27:56 -0400	[thread overview]
Message-ID: <20210705152815.1520546-40-sashal@kernel.org> (raw)
In-Reply-To: <20210705152815.1520546-1-sashal@kernel.org>

From: Thiago Rafael Becker <trbecker@gmail.com>

[ Upstream commit 6efa994e35a402ae4ae2161b6439c94b64816cee ]

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 appropriate
  	$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>
Acked-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 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 b68ba92893b6..903de7449aa3 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.30.2


  parent reply	other threads:[~2021-07-05 15:30 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-05 15:27 [PATCH AUTOSEL 5.13 01/59] HID: do not use down_interruptible() when unbinding devices Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 02/59] EDAC/ti: Add missing MODULE_DEVICE_TABLE Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 03/59] ACPI: PM: s2idle: Add missing LPS0 functions for AMD Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 04/59] ACPI: scan: Rearrange dep_unmet initialization Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 05/59] ACPI: processor idle: Fix up C-state latency if not ordered Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 06/59] hv_utils: Fix passing zero to 'PTR_ERR' warning Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 07/59] lib: vsprintf: Fix handling of number field widths in vsscanf Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 08/59] Input: goodix - platform/x86: touchscreen_dmi - Move upside down quirks to touchscreen_dmi.c Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 09/59] platform/x86: touchscreen_dmi: Add an extra entry for the upside down Goodix touchscreen on Teclast X89 tablets Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 10/59] platform/x86: touchscreen_dmi: Add info for the Goodix GT912 panel of TM800A550L tablets Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 11/59] ACPI: EC: Make more Asus laptops use ECDT _GPE Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 12/59] block_dump: remove block_dump feature in mark_inode_dirty() Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 13/59] blk-mq: grab rq->refcount before calling ->fn in blk_mq_tagset_busy_iter Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 14/59] blk-mq: clear stale request in tags->rq[] before freeing one request pool Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 15/59] fs: dlm: fix srcu read lock usage Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 16/59] fs: dlm: reconnect if socket error report occurs Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 17/59] fs: dlm: cancel work sync othercon Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 18/59] fs: dlm: fix connection tcp EOF handling Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 19/59] random32: Fix implicit truncation warning in prandom_seed_state() Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 20/59] open: don't silently ignore unknown O-flags in openat2() Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 21/59] drivers: hv: Fix missing error code in vmbus_connect() Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 22/59] fs: dlm: fix lowcomms_start error case Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 23/59] fs: dlm: fix memory leak when fenced Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 24/59] ACPICA: Fix memory leak caused by _CID repair function Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 25/59] ACPI: bus: Call kobject_put() in acpi_init() error path Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 26/59] ACPI: resources: Add checks for ACPI IRQ override Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 27/59] HID: hid-input: add Surface Go battery quirk Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 28/59] HID: sony: fix freeze when inserting ghlive ps3/wii dongles Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 29/59] block: fix race between adding/removing rq qos and normal IO Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 30/59] platform/x86: asus-nb-wmi: Revert "Drop duplicate DMI quirk structures" Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 31/59] platform/x86: asus-nb-wmi: Revert "add support for ASUS ROG Zephyrus G14 and G15" Sasha Levin
2021-07-05 17:08   ` Hans de Goede
2021-07-05 17:09     ` Hans de Goede
2021-07-09 22:50       ` Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 32/59] platform/x86: toshiba_acpi: Fix missing error code in toshiba_acpi_setup_keyboard() Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 33/59] nvme-pci: fix var. type for increasing cq_head Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 34/59] nvmet-fc: do not check for invalid target port in nvmet_fc_handle_fcp_rqst() Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 35/59] EDAC/Intel: Do not load EDAC driver when running as a guest Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 36/59] tools/power/x86/intel-speed-select: Fix uncore memory frequency display Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 37/59] PCI: hv: Add check for hyperv_initialized in init_hv_pci_drv() Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 38/59] cifs: improve fallocate emulation Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 39/59] cifs: fix check of dfs interlinks Sasha Levin
2021-07-05 15:27 ` Sasha Levin [this message]
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 41/59] smb3: fix uninitialized value for port in witness protocol move Sasha Levin
2021-07-05 15:27 ` [PATCH AUTOSEL 5.13 42/59] cifs: fix SMB1 error path in cifs_get_file_info_unix Sasha Levin

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=20210705152815.1520546-40-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lsahlber@redhat.com \
    --cc=samba-technical@lists.samba.org \
    --cc=stable@vger.kernel.org \
    --cc=stfrench@microsoft.com \
    --cc=trbecker@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).