[PATCH 1/3] CIFS: Close open handle after interrupted close

Tim Gardner tim.gardner at canonical.com
Thu Mar 11 18:18:55 UTC 2021


From: Pavel Shilovsky <pshilov at microsoft.com>

BugLink: https://bugs.launchpad.net/bugs/1918714

If Close command is interrupted before sending a request
to the server the client ends up leaking an open file
handle. This wastes server resources and can potentially
block applications that try to remove the file or any
directory containing this file.

Fix this by putting the close command into a worker queue,
so another thread retries it later.

Cc: Stable <stable at vger.kernel.org>
Tested-by: Frank Sorenson <sorenson at redhat.com>
Reviewed-by: Ronnie Sahlberg <lsahlber at redhat.com>
Signed-off-by: Pavel Shilovsky <pshilov at microsoft.com>
Signed-off-by: Steve French <stfrench at microsoft.com>
(backported from commit 9150c3adbf24d77cfba37f03639d4a908ca4ac25)
[ fs/cifs/smb2pdu.c has had significant changes, so the part of this
patch affecting fs/cifs/smb2pdu.c was pieced in where the logic looked
correct ]
Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
---

 fs/cifs/smb2misc.c  | 59 ++++++++++++++++++++++++++++++++++-----------
 fs/cifs/smb2pdu.c   |  9 ++++++-
 fs/cifs/smb2proto.h |  3 +++
 3 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index 0c6e5450ff76..d15ace6151d9 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -674,34 +674,65 @@ smb2_cancelled_close_fid(struct work_struct *work)
 	kfree(cancelled);
 }
 
+/* Caller should already has an extra reference to @tcon */
+static int
+__smb2_handle_cancelled_close(struct cifs_tcon *tcon, __u64 persistent_fid,
+			      __u64 volatile_fid)
+{
+	struct close_cancelled_open *cancelled;
+
+	cancelled = kzalloc(sizeof(*cancelled), GFP_KERNEL);
+	if (!cancelled)
+		return -ENOMEM;
+
+	cancelled->fid.persistent_fid = persistent_fid;
+	cancelled->fid.volatile_fid = volatile_fid;
+	cancelled->tcon = tcon;
+	INIT_WORK(&cancelled->work, smb2_cancelled_close_fid);
+	WARN_ON(queue_work(cifsiod_wq, &cancelled->work) == false);
+
+	return 0;
+}
+
+int
+smb2_handle_cancelled_close(struct cifs_tcon *tcon, __u64 persistent_fid,
+			    __u64 volatile_fid)
+{
+	int rc;
+
+	cifs_dbg(FYI, "%s: tc_count=%d\n", __func__, tcon->tc_count);
+	spin_lock(&cifs_tcp_ses_lock);
+	tcon->tc_count++;
+	spin_unlock(&cifs_tcp_ses_lock);
+
+	rc = __smb2_handle_cancelled_close(tcon, persistent_fid, volatile_fid);
+	if (rc)
+		cifs_put_tcon(tcon);
+
+	return rc;
+}
+
 int
 smb2_handle_cancelled_mid(char *buffer, struct TCP_Server_Info *server)
 {
 	struct smb2_sync_hdr *sync_hdr = get_sync_hdr(buffer);
 	struct smb2_create_rsp *rsp = (struct smb2_create_rsp *)buffer;
 	struct cifs_tcon *tcon;
-	struct close_cancelled_open *cancelled;
+	int rc;
 
 	if (sync_hdr->Command != SMB2_CREATE ||
 	    sync_hdr->Status != STATUS_SUCCESS)
 		return 0;
 
-	cancelled = kzalloc(sizeof(*cancelled), GFP_KERNEL);
-	if (!cancelled)
-		return -ENOMEM;
-
 	tcon = smb2_find_smb_tcon(server, sync_hdr->SessionId,
 				  sync_hdr->TreeId);
-	if (!tcon) {
-		kfree(cancelled);
+	if (!tcon)
 		return -ENOENT;
-	}
 
-	cancelled->fid.persistent_fid = rsp->PersistentFileId;
-	cancelled->fid.volatile_fid = rsp->VolatileFileId;
-	cancelled->tcon = tcon;
-	INIT_WORK(&cancelled->work, smb2_cancelled_close_fid);
-	queue_work(cifsiod_wq, &cancelled->work);
+	rc = __smb2_handle_cancelled_close(tcon, rsp->PersistentFileId,
+					   rsp->VolatileFileId);
+	if (rc)
+		cifs_put_tcon(tcon);
 
-	return 0;
+	return rc;
 }
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index ab1df4311a6d..03e4725390d4 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -2133,6 +2133,14 @@ SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
 	rsp = (struct smb2_close_rsp *)rsp_iov.iov_base;
 
 	if (rc != 0) {
+        /* retry close in a worker thread if this one is interrupted */
+        if (rc == -EINTR) {
+            int tmp_rc = smb2_handle_cancelled_close(tcon, persistent_fid,
+                                 volatile_fid);
+            if (tmp_rc)
+                cifs_dbg(VFS, "handle cancelled close fid 0x%llx returned error %d\n",
+                     persistent_fid, tmp_rc);
+        }
 		cifs_stats_fail_inc(tcon, SMB2_CLOSE_HE);
 		goto close_exit;
 	}
@@ -2147,7 +2155,6 @@ SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
 static int
 validate_buf(unsigned int offset, unsigned int buffer_length,
 	     struct smb2_hdr *hdr, unsigned int min_buf_size)
-
 {
 	unsigned int smb_len = be32_to_cpu(hdr->smb2_buf_length);
 	char *end_of_smb = smb_len + 4 /* RFC1001 length field */ + (char *)hdr;
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index 3b8e9c2e55bc..245a2595f1b1 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -180,6 +180,9 @@ extern int SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
 extern int SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
 			     const u64 persistent_fid, const u64 volatile_fid,
 			     const __u8 oplock_level);
+extern int smb2_handle_cancelled_close(struct cifs_tcon *tcon,
+				       __u64 persistent_fid,
+				       __u64 volatile_fid);
 extern int smb2_handle_cancelled_mid(char *buffer,
 					struct TCP_Server_Info *server);
 void smb2_cancelled_close_fid(struct work_struct *work);
-- 
2.17.1




More information about the kernel-team mailing list