* [cifs:for-next 31/31] fs/cifs/smb2ops.c:786 open_shroot() error: double unlock 'mutex:&tcon->crfid.fid_mutex'
@ 2019-09-13 13:55 Dan Carpenter
2019-09-13 19:46 ` Steve French
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2019-09-13 13:55 UTC (permalink / raw)
To: kbuild, Steve French, Aurelien Aptel
Cc: kbuild-all, linux-cifs, samba-technical, Pavel Shilovsky
tree: git://git.samba.org/sfrench/cifs-2.6.git for-next
head: 5fc321fb644fc787710353be11129edadd313f3a
commit: 5fc321fb644fc787710353be11129edadd313f3a [31/31] smb3: fix unmount hang in open_shroot
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
fs/cifs/smb2ops.c:786 open_shroot() error: double unlock 'mutex:&tcon->crfid.fid_mutex'
git remote add cifs git://git.samba.org/sfrench/cifs-2.6.git
git remote update cifs
git checkout 5fc321fb644fc787710353be11129edadd313f3a
vim +786 fs/cifs/smb2ops.c
fs/cifs/smb2ops.c
726 /*
727 * caller expects this func to set pfid to a valid
728 * cached root, so we copy the existing one and get a
729 * reference.
730 */
731 memcpy(pfid, tcon->crfid.fid, sizeof(*pfid));
732 kref_get(&tcon->crfid.refcount);
733
734 mutex_unlock(&tcon->crfid.fid_mutex);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Unlock (recently added)
735
736 if (rc == 0) {
737 /* close extra handle outside of crit sec */
738 SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid);
739 }
740 goto oshr_free;
741 }
742
743 /* Cached root is still invalid, continue normaly */
744
745 if (rc) {
746 if (rc == -EREMCHG) {
747 tcon->need_reconnect = true;
748 printk_once(KERN_WARNING "server share %s deleted\n",
749 tcon->treeName);
750 }
751 goto oshr_exit;
752 }
753
754 o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
755 oparms.fid->persistent_fid = o_rsp->PersistentFileId;
756 oparms.fid->volatile_fid = o_rsp->VolatileFileId;
757 #ifdef CONFIG_CIFS_DEBUG2
758 oparms.fid->mid = le64_to_cpu(o_rsp->sync_hdr.MessageId);
759 #endif /* CIFS_DEBUG2 */
760
761 memcpy(tcon->crfid.fid, pfid, sizeof(struct cifs_fid));
762 tcon->crfid.tcon = tcon;
763 tcon->crfid.is_valid = true;
764 kref_init(&tcon->crfid.refcount);
765
766 /* BB TBD check to see if oplock level check can be removed below */
767 if (o_rsp->OplockLevel == SMB2_OPLOCK_LEVEL_LEASE) {
768 kref_get(&tcon->crfid.refcount);
769 smb2_parse_contexts(server, o_rsp,
770 &oparms.fid->epoch,
771 oparms.fid->lease_key, &oplock, NULL);
772 } else
773 goto oshr_exit;
774
775 qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
776 if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info))
777 goto oshr_exit;
778 if (!smb2_validate_and_copy_iov(
779 le16_to_cpu(qi_rsp->OutputBufferOffset),
780 sizeof(struct smb2_file_all_info),
781 &rsp_iov[1], sizeof(struct smb2_file_all_info),
782 (char *)&tcon->crfid.file_all_info))
783 tcon->crfid.file_all_info_is_valid = 1;
784
785 oshr_exit:
786 mutex_unlock(&tcon->crfid.fid_mutex);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Double unlock.
787 oshr_free:
788 SMB2_open_free(&rqst[0]);
789 SMB2_query_info_free(&rqst[1]);
790 free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
791 free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
792 return rc;
793 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [cifs:for-next 31/31] fs/cifs/smb2ops.c:786 open_shroot() error: double unlock 'mutex:&tcon->crfid.fid_mutex'
2019-09-13 13:55 [cifs:for-next 31/31] fs/cifs/smb2ops.c:786 open_shroot() error: double unlock 'mutex:&tcon->crfid.fid_mutex' Dan Carpenter
@ 2019-09-13 19:46 ` Steve French
0 siblings, 0 replies; 2+ messages in thread
From: Steve French @ 2019-09-13 19:46 UTC (permalink / raw)
To: Dan Carpenter
Cc: kbuild, Steve French, Aurelien Aptel, CIFS, samba-technical,
kbuild-all, Pavel Shilovsky
[-- Attachment #1: Type: text/plain, Size: 4597 bytes --]
Updated patch, addressing the problem Dan pointed out is attached.
On Fri, Sep 13, 2019 at 8:58 AM Dan Carpenter via samba-technical
<samba-technical@lists.samba.org> wrote:
>
> tree: git://git.samba.org/sfrench/cifs-2.6.git for-next
> head: 5fc321fb644fc787710353be11129edadd313f3a
> commit: 5fc321fb644fc787710353be11129edadd313f3a [31/31] smb3: fix unmount hang in open_shroot
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> New smatch warnings:
> fs/cifs/smb2ops.c:786 open_shroot() error: double unlock 'mutex:&tcon->crfid.fid_mutex'
>
> git remote add cifs git://git.samba.org/sfrench/cifs-2.6.git
> git remote update cifs
> git checkout 5fc321fb644fc787710353be11129edadd313f3a
> vim +786 fs/cifs/smb2ops.c
>
> fs/cifs/smb2ops.c
> 726 /*
> 727 * caller expects this func to set pfid to a valid
> 728 * cached root, so we copy the existing one and get a
> 729 * reference.
> 730 */
> 731 memcpy(pfid, tcon->crfid.fid, sizeof(*pfid));
> 732 kref_get(&tcon->crfid.refcount);
> 733
> 734 mutex_unlock(&tcon->crfid.fid_mutex);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Unlock (recently added)
>
> 735
> 736 if (rc == 0) {
> 737 /* close extra handle outside of crit sec */
> 738 SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid);
> 739 }
> 740 goto oshr_free;
> 741 }
> 742
> 743 /* Cached root is still invalid, continue normaly */
> 744
> 745 if (rc) {
> 746 if (rc == -EREMCHG) {
> 747 tcon->need_reconnect = true;
> 748 printk_once(KERN_WARNING "server share %s deleted\n",
> 749 tcon->treeName);
> 750 }
> 751 goto oshr_exit;
> 752 }
> 753
> 754 o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
> 755 oparms.fid->persistent_fid = o_rsp->PersistentFileId;
> 756 oparms.fid->volatile_fid = o_rsp->VolatileFileId;
> 757 #ifdef CONFIG_CIFS_DEBUG2
> 758 oparms.fid->mid = le64_to_cpu(o_rsp->sync_hdr.MessageId);
> 759 #endif /* CIFS_DEBUG2 */
> 760
> 761 memcpy(tcon->crfid.fid, pfid, sizeof(struct cifs_fid));
> 762 tcon->crfid.tcon = tcon;
> 763 tcon->crfid.is_valid = true;
> 764 kref_init(&tcon->crfid.refcount);
> 765
> 766 /* BB TBD check to see if oplock level check can be removed below */
> 767 if (o_rsp->OplockLevel == SMB2_OPLOCK_LEVEL_LEASE) {
> 768 kref_get(&tcon->crfid.refcount);
> 769 smb2_parse_contexts(server, o_rsp,
> 770 &oparms.fid->epoch,
> 771 oparms.fid->lease_key, &oplock, NULL);
> 772 } else
> 773 goto oshr_exit;
> 774
> 775 qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
> 776 if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info))
> 777 goto oshr_exit;
> 778 if (!smb2_validate_and_copy_iov(
> 779 le16_to_cpu(qi_rsp->OutputBufferOffset),
> 780 sizeof(struct smb2_file_all_info),
> 781 &rsp_iov[1], sizeof(struct smb2_file_all_info),
> 782 (char *)&tcon->crfid.file_all_info))
> 783 tcon->crfid.file_all_info_is_valid = 1;
> 784
> 785 oshr_exit:
> 786 mutex_unlock(&tcon->crfid.fid_mutex);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Double unlock.
>
> 787 oshr_free:
> 788 SMB2_open_free(&rqst[0]);
> 789 SMB2_query_info_free(&rqst[1]);
> 790 free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
> 791 free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
> 792 return rc;
> 793 }
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
--
Thanks,
Steve
[-- Attachment #2: 0001-smb3-fix-unmount-hang-in-open_shroot.patch --]
[-- Type: text/x-patch, Size: 2765 bytes --]
From 34d8dc26eb1f37040b4819e65dfde4017ae3a281 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Thu, 12 Sep 2019 17:52:54 -0500
Subject: [PATCH] smb3: fix unmount hang in open_shroot
An earlier patch "CIFS: fix deadlock in cached root handling"
did not completely address the deadlock in open_shroot. This
patch addresses the deadlock.
In testing the recent patch:
smb3: improve handling of share deleted (and share recreated)
we were able to reproduce the open_shroot deadlock to one
of the target servers in unmount in a delete share scenario.
Fixes: 7e5a70ad88b1e ("CIFS: fix deadlock in cached root handling")
This is version 2 of this patch. An earlier version of this
patch "smb3: fix unmount hang in open_shroot" had a problem
found by Dan.
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Suggested-by: Pavel Shilovsky <pshilov@microsoft.com>
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
CC: Aurelien Aptel <aaptel@suse.com>
CC: Stable <stable@vger.kernel.org>
---
fs/cifs/smb2ops.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 3672ce0bfbaf..5776d7b0a97e 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -658,6 +658,15 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid *pfid)
return 0;
}
+ /*
+ * 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);
+
if (smb3_encryption_required(tcon))
flags |= CIFS_TRANSFORM_REQ;
@@ -679,7 +688,7 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid *pfid)
rc = SMB2_open_init(tcon, &rqst[0], &oplock, &oparms, &utf16_path);
if (rc)
- goto oshr_exit;
+ goto oshr_free;
smb2_set_next_command(tcon, &rqst[0]);
memset(&qi_iov, 0, sizeof(qi_iov));
@@ -692,18 +701,10 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid *pfid)
sizeof(struct smb2_file_all_info) +
PATH_MAX * 2, 0, NULL);
if (rc)
- goto oshr_exit;
+ goto oshr_free;
smb2_set_related(&rqst[1]);
- /*
- * 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 = compound_send_recv(xid, ses, flags, 2, rqst,
resp_buftype, rsp_iov);
mutex_lock(&tcon->crfid.fid_mutex);
--
2.20.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-09-13 19:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-13 13:55 [cifs:for-next 31/31] fs/cifs/smb2ops.c:786 open_shroot() error: double unlock 'mutex:&tcon->crfid.fid_mutex' Dan Carpenter
2019-09-13 19:46 ` Steve French
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).