All of lore.kernel.org
 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:29 UTC|newest]

Thread overview: 53+ 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   ` [Cluster-devel] " 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   ` [Cluster-devel] " 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   ` [Cluster-devel] " 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   ` [Cluster-devel] " 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   ` [Cluster-devel] " 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   ` [Cluster-devel] " 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   ` 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   ` 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 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.