Linux-CIFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/3] CIFS: Close open handle after interrupted close
@ 2019-11-21 19:35 Pavel Shilovsky
  2019-11-21 19:35 ` [PATCH 2/3] CIFS: Fix NULL pointer dereference in mid callback Pavel Shilovsky
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Pavel Shilovsky @ 2019-11-21 19:35 UTC (permalink / raw)
  To: Steve French, Ronnie Sahlberg, Frank Sorenson, linux-cifs

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@vger.kernel.org>
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/smb2misc.c  | 59 ++++++++++++++++++++++++++++++++++-----------
 fs/cifs/smb2pdu.c   | 16 +++++++++++-
 fs/cifs/smb2proto.h |  3 +++
 3 files changed, 63 insertions(+), 15 deletions(-)

diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index 527c9efd3de0..80a8f4b2daab 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -725,36 +725,67 @@ 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 = (struct smb2_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 0aa40129dfb5..2f541e9efba1 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -2942,7 +2942,21 @@ int
 SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
 	   u64 persistent_fid, u64 volatile_fid)
 {
-	return SMB2_close_flags(xid, tcon, persistent_fid, volatile_fid, 0);
+	int rc;
+	int tmp_rc;
+
+	rc = SMB2_close_flags(xid, tcon, persistent_fid, volatile_fid, 0);
+
+	/* retry close in a worker thread if this one is interrupted */
+	if (rc == -EINTR) {
+		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);
+	}
+
+	return rc;
 }
 
 int
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index 07ca72486cfa..e239f98093a9 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -203,6 +203,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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/3] CIFS: Fix NULL pointer dereference in mid callback
  2019-11-21 19:35 [PATCH 1/3] CIFS: Close open handle after interrupted close Pavel Shilovsky
@ 2019-11-21 19:35 ` Pavel Shilovsky
  2019-11-21 19:35 ` [PATCH 3/3] CIFS: Do not miss cancelled OPEN responses Pavel Shilovsky
  2019-11-21 22:00 ` [PATCH 1/3] CIFS: Close open handle after interrupted close Steve French
  2 siblings, 0 replies; 5+ messages in thread
From: Pavel Shilovsky @ 2019-11-21 19:35 UTC (permalink / raw)
  To: Steve French, Ronnie Sahlberg, Frank Sorenson, linux-cifs

There is a race between a system call processing thread
and the demultiplex thread when mid->resp_buf becomes NULL
and later is being accessed to get credits. It happens when
the 1st thread wakes up before a mid callback is called in
the 2nd one but the mid state has already been set to
MID_RESPONSE_RECEIVED. This causes NULL pointer dereference
in mid callback.

Fix this by saving credits from the response before we
update the mid state and then use this value in the mid
callback rather then accessing a response buffer.

Cc: Stable <stable@vger.kernel.org>
Fixes: ee258d79159afed5 ("CIFS: Move credit processing to mid callbacks for SMB3")
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/cifsglob.h |  1 +
 fs/cifs/connect.c  | 15 +++++++++++++++
 fs/cifs/smb2ops.c  |  8 +-------
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 12b356b3799b..128364af4c37 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1510,6 +1510,7 @@ struct mid_q_entry {
 	struct TCP_Server_Info *server;	/* server corresponding to this mid */
 	__u64 mid;		/* multiplex id */
 	__u16 credits;		/* number of credits consumed by this mid */
+	__u16 credits_received;	/* number of credits from the response */
 	__u32 pid;		/* process id */
 	__u32 sequence_number;  /* for CIFS signing */
 	unsigned long when_alloc;  /* when mid was created */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 8995c03056e3..e63d16d8048a 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -897,6 +897,20 @@ dequeue_mid(struct mid_q_entry *mid, bool malformed)
 	spin_unlock(&GlobalMid_Lock);
 }
 
+static unsigned int
+smb2_get_credits_from_hdr(char *buffer, struct TCP_Server_Info *server)
+{
+	struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)buffer;
+
+	/*
+	 * SMB1 does not use credits.
+	 */
+	if (server->vals->header_preamble_size)
+		return 0;
+
+	return le16_to_cpu(shdr->CreditRequest);
+}
+
 static void
 handle_mid(struct mid_q_entry *mid, struct TCP_Server_Info *server,
 	   char *buf, int malformed)
@@ -904,6 +918,7 @@ handle_mid(struct mid_q_entry *mid, struct TCP_Server_Info *server,
 	if (server->ops->check_trans2 &&
 	    server->ops->check_trans2(mid, server, buf, malformed))
 		return;
+	mid->credits_received = smb2_get_credits_from_hdr(buf, server);
 	mid->resp_buf = buf;
 	mid->large_buf = server->large_buf;
 	/* Was previous buf put in mpx struct for multi-rsp? */
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 3bbb65e58dd6..5a13687bf547 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -151,13 +151,7 @@ smb2_get_credits_field(struct TCP_Server_Info *server, const int optype)
 static unsigned int
 smb2_get_credits(struct mid_q_entry *mid)
 {
-	struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)mid->resp_buf;
-
-	if (mid->mid_state == MID_RESPONSE_RECEIVED
-	    || mid->mid_state == MID_RESPONSE_MALFORMED)
-		return le16_to_cpu(shdr->CreditRequest);
-
-	return 0;
+	return mid->credits_received;
 }
 
 static int
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/3] CIFS: Do not miss cancelled OPEN responses
  2019-11-21 19:35 [PATCH 1/3] CIFS: Close open handle after interrupted close Pavel Shilovsky
  2019-11-21 19:35 ` [PATCH 2/3] CIFS: Fix NULL pointer dereference in mid callback Pavel Shilovsky
@ 2019-11-21 19:35 ` Pavel Shilovsky
  2019-11-21 19:45   ` ronnie sahlberg
  2019-11-21 22:00 ` [PATCH 1/3] CIFS: Close open handle after interrupted close Steve French
  2 siblings, 1 reply; 5+ messages in thread
From: Pavel Shilovsky @ 2019-11-21 19:35 UTC (permalink / raw)
  To: Steve French, Ronnie Sahlberg, Frank Sorenson, linux-cifs

When an OPEN command is cancelled we mark a mid as
cancelled and let the demultiplex thread process it
by closing an open handle. The problem is there is
a race between a system call thread and the demultiplex
thread and there may be a situation when the mid has
been already processed before it is set as cancelled.

Fix this by processing cancelled requests when mids
are being destroyed which means that there is only
one thread referencing a particular mid. Also set
mids as cancelled unconditionally on their state.

Cc: Stable <stable@vger.kernel.org>
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/connect.c   |  6 ------
 fs/cifs/transport.c | 10 ++++++++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index e63d16d8048a..59feb2de389e 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1229,12 +1229,6 @@ cifs_demultiplex_thread(void *p)
 		for (i = 0; i < num_mids; i++) {
 			if (mids[i] != NULL) {
 				mids[i]->resp_buf_size = server->pdu_size;
-				if ((mids[i]->mid_flags & MID_WAIT_CANCELLED) &&
-				    mids[i]->mid_state == MID_RESPONSE_RECEIVED &&
-				    server->ops->handle_cancelled_mid)
-					server->ops->handle_cancelled_mid(
-							mids[i]->resp_buf,
-							server);
 
 				if (!mids[i]->multiRsp || mids[i]->multiEnd)
 					mids[i]->callback(mids[i]);
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index bb52751ba783..987ffcd5ca3a 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -93,8 +93,14 @@ static void _cifs_mid_q_entry_release(struct kref *refcount)
 	__u16 smb_cmd = le16_to_cpu(midEntry->command);
 	unsigned long now;
 	unsigned long roundtrip_time;
-	struct TCP_Server_Info *server = midEntry->server;
 #endif
+	struct TCP_Server_Info *server = midEntry->server;
+
+	if (midEntry->resp_buf && (midEntry->mid_flags & MID_WAIT_CANCELLED) &&
+	    midEntry->mid_state == MID_RESPONSE_RECEIVED &&
+	    server->ops->handle_cancelled_mid)
+		server->ops->handle_cancelled_mid(midEntry->resp_buf, server);
+
 	midEntry->mid_state = MID_FREE;
 	atomic_dec(&midCount);
 	if (midEntry->large_buf)
@@ -1115,8 +1121,8 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 				 midQ[i]->mid, le16_to_cpu(midQ[i]->command));
 			send_cancel(server, &rqst[i], midQ[i]);
 			spin_lock(&GlobalMid_Lock);
+			midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
 			if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED) {
-				midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
 				midQ[i]->callback = cifs_cancelled_callback;
 				cancelled_mid[i] = true;
 				credits[i].value = 0;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] CIFS: Do not miss cancelled OPEN responses
  2019-11-21 19:35 ` [PATCH 3/3] CIFS: Do not miss cancelled OPEN responses Pavel Shilovsky
@ 2019-11-21 19:45   ` ronnie sahlberg
  0 siblings, 0 replies; 5+ messages in thread
From: ronnie sahlberg @ 2019-11-21 19:45 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: Steve French, Ronnie Sahlberg, Frank Sorenson, linux-cifs

Very nice.

Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com
>

On Fri, Nov 22, 2019 at 5:35 AM Pavel Shilovsky <piastryyy@gmail.com> wrote:
>
> When an OPEN command is cancelled we mark a mid as
> cancelled and let the demultiplex thread process it
> by closing an open handle. The problem is there is
> a race between a system call thread and the demultiplex
> thread and there may be a situation when the mid has
> been already processed before it is set as cancelled.
>
> Fix this by processing cancelled requests when mids
> are being destroyed which means that there is only
> one thread referencing a particular mid. Also set
> mids as cancelled unconditionally on their state.
>
> Cc: Stable <stable@vger.kernel.org>
> Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
> ---
>  fs/cifs/connect.c   |  6 ------
>  fs/cifs/transport.c | 10 ++++++++--
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index e63d16d8048a..59feb2de389e 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1229,12 +1229,6 @@ cifs_demultiplex_thread(void *p)
>                 for (i = 0; i < num_mids; i++) {
>                         if (mids[i] != NULL) {
>                                 mids[i]->resp_buf_size = server->pdu_size;
> -                               if ((mids[i]->mid_flags & MID_WAIT_CANCELLED) &&
> -                                   mids[i]->mid_state == MID_RESPONSE_RECEIVED &&
> -                                   server->ops->handle_cancelled_mid)
> -                                       server->ops->handle_cancelled_mid(
> -                                                       mids[i]->resp_buf,
> -                                                       server);
>
>                                 if (!mids[i]->multiRsp || mids[i]->multiEnd)
>                                         mids[i]->callback(mids[i]);
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index bb52751ba783..987ffcd5ca3a 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -93,8 +93,14 @@ static void _cifs_mid_q_entry_release(struct kref *refcount)
>         __u16 smb_cmd = le16_to_cpu(midEntry->command);
>         unsigned long now;
>         unsigned long roundtrip_time;
> -       struct TCP_Server_Info *server = midEntry->server;
>  #endif
> +       struct TCP_Server_Info *server = midEntry->server;
> +
> +       if (midEntry->resp_buf && (midEntry->mid_flags & MID_WAIT_CANCELLED) &&
> +           midEntry->mid_state == MID_RESPONSE_RECEIVED &&
> +           server->ops->handle_cancelled_mid)
> +               server->ops->handle_cancelled_mid(midEntry->resp_buf, server);
> +
>         midEntry->mid_state = MID_FREE;
>         atomic_dec(&midCount);
>         if (midEntry->large_buf)
> @@ -1115,8 +1121,8 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>                                  midQ[i]->mid, le16_to_cpu(midQ[i]->command));
>                         send_cancel(server, &rqst[i], midQ[i]);
>                         spin_lock(&GlobalMid_Lock);
> +                       midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
>                         if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED) {
> -                               midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
>                                 midQ[i]->callback = cifs_cancelled_callback;
>                                 cancelled_mid[i] = true;
>                                 credits[i].value = 0;
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] CIFS: Close open handle after interrupted close
  2019-11-21 19:35 [PATCH 1/3] CIFS: Close open handle after interrupted close Pavel Shilovsky
  2019-11-21 19:35 ` [PATCH 2/3] CIFS: Fix NULL pointer dereference in mid callback Pavel Shilovsky
  2019-11-21 19:35 ` [PATCH 3/3] CIFS: Do not miss cancelled OPEN responses Pavel Shilovsky
@ 2019-11-21 22:00 ` Steve French
  2 siblings, 0 replies; 5+ messages in thread
From: Steve French @ 2019-11-21 22:00 UTC (permalink / raw)
  To: Pavel Shilovsky, Aurélien Aptel, Paulo Alcantara
  Cc: Ronnie Sahlberg, Frank Sorenson, linux-cifs

Merged these three (and one of Aurelien's recent patches for
multichannel - need updates to that, also waiting on Paulo's DFS fix)
into cifs-2.6.git for-next (and also the buildbot's gitub for-next
branch)


On Thu, Nov 21, 2019 at 1:35 PM Pavel Shilovsky <piastryyy@gmail.com> wrote:
>
> 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@vger.kernel.org>
> Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
> ---
>  fs/cifs/smb2misc.c  | 59 ++++++++++++++++++++++++++++++++++-----------
>  fs/cifs/smb2pdu.c   | 16 +++++++++++-
>  fs/cifs/smb2proto.h |  3 +++
>  3 files changed, 63 insertions(+), 15 deletions(-)
>
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index 527c9efd3de0..80a8f4b2daab 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -725,36 +725,67 @@ 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 = (struct smb2_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 0aa40129dfb5..2f541e9efba1 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -2942,7 +2942,21 @@ int
>  SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
>            u64 persistent_fid, u64 volatile_fid)
>  {
> -       return SMB2_close_flags(xid, tcon, persistent_fid, volatile_fid, 0);
> +       int rc;
> +       int tmp_rc;
> +
> +       rc = SMB2_close_flags(xid, tcon, persistent_fid, volatile_fid, 0);
> +
> +       /* retry close in a worker thread if this one is interrupted */
> +       if (rc == -EINTR) {
> +               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);
> +       }
> +
> +       return rc;
>  }
>
>  int
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index 07ca72486cfa..e239f98093a9 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -203,6 +203,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
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-21 19:35 [PATCH 1/3] CIFS: Close open handle after interrupted close Pavel Shilovsky
2019-11-21 19:35 ` [PATCH 2/3] CIFS: Fix NULL pointer dereference in mid callback Pavel Shilovsky
2019-11-21 19:35 ` [PATCH 3/3] CIFS: Do not miss cancelled OPEN responses Pavel Shilovsky
2019-11-21 19:45   ` ronnie sahlberg
2019-11-21 22:00 ` [PATCH 1/3] CIFS: Close open handle after interrupted close Steve French

Linux-CIFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-cifs/0 linux-cifs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-cifs linux-cifs/ https://lore.kernel.org/linux-cifs \
		linux-cifs@vger.kernel.org
	public-inbox-index linux-cifs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-cifs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git