Stable Archive on lore.kernel.org
 help / color / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Dave Wysochanski <dwysocha@redhat.com>,
	Ronnie Sahlberg <lsahlber@redhat.com>,
	Pavel Shilovsky <pshilov@microsoft.com>,
	Sasha Levin <sashal@kernel.org>
Subject: [PATCH 4.14 21/62] cifs: Fix cifsInodeInfo lock_sem deadlock when reconnect occurs
Date: Fri,  8 Nov 2019 19:50:09 +0100
Message-ID: <20191108174736.760350624@linuxfoundation.org> (raw)
In-Reply-To: <20191108174719.228826381@linuxfoundation.org>

From: Dave Wysochanski <dwysocha@redhat.com>

[ Upstream commit d46b0da7a33dd8c99d969834f682267a45444ab3 ]

There's a deadlock that is possible and can easily be seen with
a test where multiple readers open/read/close of the same file
and a disruption occurs causing reconnect.  The deadlock is due
a reader thread inside cifs_strict_readv calling down_read and
obtaining lock_sem, and then after reconnect inside
cifs_reopen_file calling down_read a second time.  If in
between the two down_read calls, a down_write comes from
another process, deadlock occurs.

        CPU0                    CPU1
        ----                    ----
cifs_strict_readv()
 down_read(&cifsi->lock_sem);
                               _cifsFileInfo_put
                                  OR
                               cifs_new_fileinfo
                                down_write(&cifsi->lock_sem);
cifs_reopen_file()
 down_read(&cifsi->lock_sem);

Fix the above by changing all down_write(lock_sem) calls to
down_write_trylock(lock_sem)/msleep() loop, which in turn
makes the second down_read call benign since it will never
block behind the writer while holding lock_sem.

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
Suggested-by: Ronnie Sahlberg <lsahlber@redhat.com>
Reviewed--by: Ronnie Sahlberg <lsahlber@redhat.com>
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/cifs/cifsglob.h  |  5 +++++
 fs/cifs/cifsproto.h |  1 +
 fs/cifs/file.c      | 23 +++++++++++++++--------
 fs/cifs/smb2file.c  |  2 +-
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 7b7ab10a9db18..600bb838c15b8 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1210,6 +1210,11 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
 struct cifsInodeInfo {
 	bool can_cache_brlcks;
 	struct list_head llist;	/* locks helb by this inode */
+	/*
+	 * NOTE: Some code paths call down_read(lock_sem) twice, so
+	 * we must always use use cifs_down_write() instead of down_write()
+	 * for this semaphore to avoid deadlocks.
+	 */
 	struct rw_semaphore lock_sem;	/* protect the fields above */
 	/* BB add in lists for dirty pages i.e. write caching info for oplock */
 	struct list_head openFileList;
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index ccdb42f71b2e8..3a7fb8e750e97 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -149,6 +149,7 @@ extern int cifs_unlock_range(struct cifsFileInfo *cfile,
 			     struct file_lock *flock, const unsigned int xid);
 extern int cifs_push_mandatory_locks(struct cifsFileInfo *cfile);
 
+extern void cifs_down_write(struct rw_semaphore *sem);
 extern struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid,
 					      struct file *file,
 					      struct tcon_link *tlink,
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 71a960da7cce1..40f22932343c6 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -280,6 +280,13 @@ cifs_has_mand_locks(struct cifsInodeInfo *cinode)
 	return has_locks;
 }
 
+void
+cifs_down_write(struct rw_semaphore *sem)
+{
+	while (!down_write_trylock(sem))
+		msleep(10);
+}
+
 struct cifsFileInfo *
 cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
 		  struct tcon_link *tlink, __u32 oplock)
@@ -305,7 +312,7 @@ cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
 	INIT_LIST_HEAD(&fdlocks->locks);
 	fdlocks->cfile = cfile;
 	cfile->llist = fdlocks;
-	down_write(&cinode->lock_sem);
+	cifs_down_write(&cinode->lock_sem);
 	list_add(&fdlocks->llist, &cinode->llist);
 	up_write(&cinode->lock_sem);
 
@@ -457,7 +464,7 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_handler)
 	 * Delete any outstanding lock records. We'll lose them when the file
 	 * is closed anyway.
 	 */
-	down_write(&cifsi->lock_sem);
+	cifs_down_write(&cifsi->lock_sem);
 	list_for_each_entry_safe(li, tmp, &cifs_file->llist->locks, llist) {
 		list_del(&li->llist);
 		cifs_del_lock_waiters(li);
@@ -1011,7 +1018,7 @@ static void
 cifs_lock_add(struct cifsFileInfo *cfile, struct cifsLockInfo *lock)
 {
 	struct cifsInodeInfo *cinode = CIFS_I(d_inode(cfile->dentry));
-	down_write(&cinode->lock_sem);
+	cifs_down_write(&cinode->lock_sem);
 	list_add_tail(&lock->llist, &cfile->llist->locks);
 	up_write(&cinode->lock_sem);
 }
@@ -1033,7 +1040,7 @@ cifs_lock_add_if(struct cifsFileInfo *cfile, struct cifsLockInfo *lock,
 
 try_again:
 	exist = false;
-	down_write(&cinode->lock_sem);
+	cifs_down_write(&cinode->lock_sem);
 
 	exist = cifs_find_lock_conflict(cfile, lock->offset, lock->length,
 					lock->type, &conf_lock, CIFS_LOCK_OP);
@@ -1055,7 +1062,7 @@ cifs_lock_add_if(struct cifsFileInfo *cfile, struct cifsLockInfo *lock,
 					(lock->blist.next == &lock->blist));
 		if (!rc)
 			goto try_again;
-		down_write(&cinode->lock_sem);
+		cifs_down_write(&cinode->lock_sem);
 		list_del_init(&lock->blist);
 	}
 
@@ -1108,7 +1115,7 @@ cifs_posix_lock_set(struct file *file, struct file_lock *flock)
 		return rc;
 
 try_again:
-	down_write(&cinode->lock_sem);
+	cifs_down_write(&cinode->lock_sem);
 	if (!cinode->can_cache_brlcks) {
 		up_write(&cinode->lock_sem);
 		return rc;
@@ -1314,7 +1321,7 @@ cifs_push_locks(struct cifsFileInfo *cfile)
 	int rc = 0;
 
 	/* we are going to update can_cache_brlcks here - need a write access */
-	down_write(&cinode->lock_sem);
+	cifs_down_write(&cinode->lock_sem);
 	if (!cinode->can_cache_brlcks) {
 		up_write(&cinode->lock_sem);
 		return rc;
@@ -1505,7 +1512,7 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock,
 	if (!buf)
 		return -ENOMEM;
 
-	down_write(&cinode->lock_sem);
+	cifs_down_write(&cinode->lock_sem);
 	for (i = 0; i < 2; i++) {
 		cur = buf;
 		num = 0;
diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
index 1add404618f06..2c809233084bb 100644
--- a/fs/cifs/smb2file.c
+++ b/fs/cifs/smb2file.c
@@ -139,7 +139,7 @@ smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock,
 
 	cur = buf;
 
-	down_write(&cinode->lock_sem);
+	cifs_down_write(&cinode->lock_sem);
 	list_for_each_entry_safe(li, tmp, &cfile->llist->locks, llist) {
 		if (flock->fl_start > li->offset ||
 		    (flock->fl_start + length) <
-- 
2.20.1




  parent reply index

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-08 18:49 [PATCH 4.14 00/62] 4.14.153-stable review Greg Kroah-Hartman
2019-11-08 18:49 ` [PATCH 4.14 01/62] arm64: dts: Fix gpio to pinmux mapping Greg Kroah-Hartman
2019-11-08 18:49 ` [PATCH 4.14 02/62] regulator: ti-abb: Fix timeout in ti_abb_wait_txdone/ti_abb_clear_all_txdone Greg Kroah-Hartman
2019-11-08 18:49 ` [PATCH 4.14 03/62] regulator: pfuze100-regulator: Variable "val" in pfuze100_regulator_probe() could be uninitialized Greg Kroah-Hartman
2019-11-08 18:49 ` [PATCH 4.14 04/62] ASoC: wm_adsp: Dont generate kcontrols without READ flags Greg Kroah-Hartman
2019-11-08 18:49 ` [PATCH 4.14 05/62] ASoc: rockchip: i2s: Fix RPM imbalance Greg Kroah-Hartman
2019-11-08 18:49 ` [PATCH 4.14 06/62] ARM: dts: logicpd-torpedo-som: Remove twl_keypad Greg Kroah-Hartman
2019-11-08 18:49 ` [PATCH 4.14 07/62] pinctrl: ns2: Fix off by one bugs in ns2_pinmux_enable() Greg Kroah-Hartman
2019-11-08 18:49 ` [PATCH 4.14 08/62] ARM: mm: fix alignment handler faults under memory pressure Greg Kroah-Hartman
2019-11-08 18:49 ` [PATCH 4.14 09/62] scsi: scsi_dh_alua: handle RTPG sense code correctly during state transitions Greg Kroah-Hartman
2019-11-08 18:49 ` [PATCH 4.14 10/62] scsi: sni_53c710: fix compilation error Greg Kroah-Hartman
2019-11-08 18:49 ` [PATCH 4.14 11/62] scsi: fix kconfig dependency warning related to 53C700_LE_ON_BE Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 12/62] ARM: dts: imx7s: Correct GPTs ipg clock source Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 13/62] perf c2c: Fix memory leak in build_cl_output() Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 14/62] perf kmem: Fix memory leak in compact_gfp_flags() Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 15/62] ARM: davinci: dm365: Fix McBSP dma_slave_map entry Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 16/62] scsi: target: core: Do not overwrite CDB byte 1 Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 17/62] ARM: 8926/1: v7m: remove register save to stack before svc Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 18/62] of: unittest: fix memory leak in unittest_data_add Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 19/62] MIPS: bmips: mark exception vectors as char arrays Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 20/62] i2c: stm32f7: remove warning when compiling with W=1 Greg Kroah-Hartman
2019-11-08 18:50 ` Greg Kroah-Hartman [this message]
2019-11-08 18:50 ` [PATCH 4.14 22/62] nbd: handle racing with errored out commands Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 23/62] cxgb4: fix panic when attaching to ULD fail Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 24/62] dccp: do not leak jiffies on the wire Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 25/62] net: annotate accesses to sk->sk_incoming_cpu Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 26/62] net: annotate lockless accesses to sk->sk_napi_id Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 27/62] net: dsa: bcm_sf2: Fix IMP setup for port different than 8 Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 28/62] net: ethernet: ftgmac100: Fix DMA coherency issue with SW checksum Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 29/62] net: fix sk_page_frag() recursion from memory reclaim Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 30/62] net: hisilicon: Fix ping latency when deal with high throughput Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 31/62] net/mlx4_core: Dynamically set guaranteed amount of counters per VF Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 32/62] net: Zeroing the structure ethtool_wolinfo in ethtool_get_wol() Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 33/62] selftests: net: reuseport_dualstack: fix uninitalized parameter Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 34/62] udp: fix data-race in udp_set_dev_scratch() Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 35/62] net: add READ_ONCE() annotation in __skb_wait_for_more_packets() Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 36/62] net/mlx5e: Fix handling of compressed CQEs in case of low NAPI budget Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 37/62] net: dsa: b53: Do not clear existing mirrored port mask Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 38/62] net: usb: lan78xx: Connect PHY before registering MAC Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 39/62] r8152: add device id for Lenovo ThinkPad USB-C Dock Gen 2 Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 40/62] net: dsa: fix switch tree list Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 41/62] net: bcmgenet: reset 40nm EPHY on energy detect Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 42/62] net: add skb_queue_empty_lockless() Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 43/62] udp: use skb_queue_empty_lockless() Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 44/62] net: use skb_queue_empty_lockless() in poll() handlers Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 45/62] net: use skb_queue_empty_lockless() in busy poll contexts Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 46/62] vxlan: check tun_info options_len properly Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 47/62] erspan: fix the tun_info options_len check for erspan Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 48/62] inet: stop leaking jiffies on the wire Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 49/62] net/flow_dissector: switch to siphash Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 50/62] dmaengine: qcom: bam_dma: Fix resource leak Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 51/62] sched/wake_q: Fix wakeup ordering for wake_q Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 52/62] kbuild: use -fmacro-prefix-map to make __FILE__ a relative path Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 53/62] kbuild: add -fcf-protection=none when using retpoline flags Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 54/62] platform/x86: pmc_atom: Add Siemens SIMATIC IPC227E to critclk_systems DMI table Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 55/62] iio: adc: stm32-adc: move registers definitions Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 56/62] iio: adc: stm32-adc: fix a race when using several adcs with dma and irq Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 57/62] powerpc/mm: Fixup tlbie vs store ordering issue on POWER9 Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 58/62] powerpc/book3s64/mm: Dont do tlbie fixup for some hardware revisions Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 59/62] powerpc/book3s64/radix: Rename CPU_FTR_P9_TLBIE_BUG feature flag Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 60/62] powerpc/mm: Fixup tlbie vs mtpidr/mtlpidr ordering issue on POWER9 Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 61/62] selftests/powerpc: Add test case for tlbie vs mtpidr ordering issue Greg Kroah-Hartman
2019-11-08 18:50 ` [PATCH 4.14 62/62] selftests/powerpc: Fix compile error on tlbie_test due to newer gcc Greg Kroah-Hartman
2019-11-09  1:57 ` [PATCH 4.14 00/62] 4.14.153-stable review kernelci.org bot
2019-11-09 10:19 ` Naresh Kamboju
2019-11-09 15:40 ` Guenter Roeck

Reply instructions:

You may reply publically 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=20191108174736.760350624@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=dwysocha@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lsahlber@redhat.com \
    --cc=pshilov@microsoft.com \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    /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

Stable Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/stable/0 stable/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 stable stable/ https://lore.kernel.org/stable \
		stable@vger.kernel.org
	public-inbox-index stable

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.stable


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