All of lore.kernel.org
 help / color / mirror / Atom feed
From: Aurelien Aptel <aaptel@suse.com>
To: linux-cifs@vger.kernel.org
Cc: piastryyy@gmail.com, Aurelien Aptel <aaptel@suse.com>
Subject: [PATCH v1 4.20.y stable] CIFS: fix deadlock in cached root handling
Date: Sat,  6 Jul 2019 21:17:31 +0200	[thread overview]
Message-ID: <20190706191731.31150-1-aaptel@suse.com> (raw)
In-Reply-To: <CAKywueScmnouPGgaEkpH3pfXCzSnddz-qCZqYPjBQLGX5c__ng@mail.gmail.com>

Prevent deadlock between open_shroot() and
cifs_mark_open_files_invalid() by releasing the lock before entering
SMB2_open, taking it again after and checking if we still need to use
the result.

Link: https://lore.kernel.org/linux-cifs/684ed01c-cbca-2716-bc28-b0a59a0f8521@prodrive-technologies.com/T/#u
Fixes: 3d4ef9a15343 ("smb3: fix redundant opens on root")
Signed-off-by: Aurelien Aptel <aaptel@suse.com>
---

This patch applies on top of the 4.20 stable tree.

Please review this carefuly, I have not tested it.

 XXX: do we need to call SMB2_close() when the work was already done
      concurrently?  if yes we need to do it outside the critical
      section otherwise we hit the same issue again

 fs/cifs/smb2ops.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index aa71e620f3cd..55991e43d74f 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -609,7 +609,27 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid *pfid)
 	oparams.fid = pfid;
 	oparams.reconnect = false;
 
+	/*
+	 * We do not hold the lock for the open because in case
+	 * SMB2_open needs to reconnect, it will end up calling
+	 * cifs_mark_open_files_invalid() which takes the lock again
+	 * thus causing a deadlock
+	 */
+	mutex_unlock(&tcon->crfid.fid_mutex);
 	rc = SMB2_open(xid, &oparams, &srch_path, &oplock, NULL, NULL, NULL);
+	mutex_lock(&tcon->crfid.fid_mutex);
+
+	/*
+	 * Now we need to check again as the cached root might have
+	 * been successfully re-opened from a concurrent process
+	 */
+
+	if (tcon->crfid.is_valid) {
+		/* work was already done */
+		rc = 0;
+		goto out;
+	}
+
 	if (rc == 0) {
 		memcpy(tcon->crfid.fid, pfid, sizeof(struct cifs_fid));
 		tcon->crfid.tcon = tcon;
@@ -617,6 +637,8 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid *pfid)
 		kref_init(&tcon->crfid.refcount);
 		kref_get(&tcon->crfid.refcount);
 	}
+
+out:
 	mutex_unlock(&tcon->crfid.fid_mutex);
 	return rc;
 }
-- 
2.16.4


       reply	other threads:[~2019-07-06 19:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAKywueScmnouPGgaEkpH3pfXCzSnddz-qCZqYPjBQLGX5c__ng@mail.gmail.com>
2019-07-06 19:17 ` Aurelien Aptel [this message]
2019-07-06 19:30   ` [PATCH v1 4.20.y stable] CIFS: fix deadlock in cached root handling Pavel Shilovsky
2019-07-06 20:11     ` [PATCH v2 " Aurelien Aptel

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=20190706191731.31150-1-aaptel@suse.com \
    --to=aaptel@suse.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=piastryyy@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.