Skip to contentSkip to conversation

4.9.47-stable review

PATCH In review Greg Kroah-Hartman 6 participants 19 messages Aug 31, 2017 – Sep 3, 2017 Stable branch

lore
Share & tools
lore

Greg Kroah-Hartman

author maintainer
#0 lore raw

cover 4.9.47-stable review

This is the start of the stable review cycle for the 4.9.47 release. There are 12 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Sat Sep 2 15:42:31 UTC 2017. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

	kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.47-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-4.9.y
and the diffstat can be found below.

thanks,

greg k-h

-------------
Pseudo-Shortlog of commits:

Greg Kroah-Hartman <[email protected]>

    Linux 4.9.47-rc1

Greg Kroah-Hartman <[email protected]>
lz4: fix bogus gcc warning

Hannes Reinecke <[email protected]>
scsi: sg: reset 'res_in_use' after unlinking reserved array

Hannes Reinecke <[email protected]>
scsi: sg: protect accesses to 'reserved' page array

Waiman Long <[email protected]>
locking/spinlock/debug: Remove spinlock lockup detection code

Dave Martin <[email protected]>
arm64: fpsimd: Prevent registers leaking across exec

Arnd Bergmann <[email protected]>
x86/io: Add "memory" clobber to insb/insw/insl/outsb/outsw/outsl

Mark Rutland <[email protected]>
arm64: mm: abort uaccess retries upon fatal signal

Suzuki K Poulose <[email protected]>
kvm: arm/arm64: Fix race in resetting stage2 PGD

Martin Liska <[email protected]>
gcov: support GCC 7.1

Arnd Bergmann <[email protected]>

    staging: wilc1000: simplify vif[i]->ndev accesses

Arnd Bergmann <[email protected]>
scsi: isci: avoid array subscript warning

Jiri Slaby <[email protected]>
p54: memset(0) whole array

-------------

Diffstat:

 Makefile                                 |  4 +-
 arch/arm/kvm/mmu.c                       | 16 +++---
 arch/arm64/kernel/fpsimd.c               |  2 +
 arch/arm64/mm/fault.c                    |  5 +-
 arch/x86/include/asm/io.h                |  4 +-
 drivers/net/wireless/intersil/p54/fwio.c |  2 +-
 drivers/scsi/isci/remote_node_context.c  |  3 ++
 drivers/scsi/sg.c                        | 49 ++++++++++--------
 drivers/staging/wilc1000/linux_wlan.c    | 34 ++++---------
 kernel/gcov/base.c                       |  6 +++
 kernel/gcov/gcc_4_7.c                    |  4 +-
 kernel/locking/spinlock_debug.c          | 86 ++------------------------------
 lib/lz4/lz4hc_compress.c                 |  2 +-
 13 files changed, 74 insertions(+), 143 deletions(-)
author maintainer
#1 lore raw

PATCH 1/12 p54: memset(0) whole array

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Jiri Slaby <[email protected]>

commit 6f17581788206444cbbcdbc107498f85e9765e3d upstream.

gcc 7 complains:
drivers/net/wireless/intersil/p54/fwio.c: In function 'p54_scan':
drivers/net/wireless/intersil/p54/fwio.c:491:4: warning: 'memset' used with length equal to number of elements without multiplication by element size [-Wmemset-elt-size]

Fix that by passing the correct size to memset.

---
 drivers/net/wireless/intersil/p54/fwio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/intersil/p54/fwio.c +1 -1
@@ -488,7 +488,7 @@ int p54_scan(struct p54_common *priv, u1
 
 			entry += sizeof(__le16);
 			chan->pa_points_per_curve = 8;
-			memset(chan->curve_data, 0, sizeof(*chan->curve_data));
+			memset(chan->curve_data, 0, sizeof(chan->curve_data));
 			memcpy(chan->curve_data, entry,
 			       sizeof(struct p54_pa_curve_data_sample) *
 			       min((u8)8, curve_data->points_per_channel));
#2 lore raw

Re: [PATCH 4.9 01/12] p54: memset(0) whole array

Quoted 29 lines · Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Jiri Slaby <[email protected]>

commit 6f17581788206444cbbcdbc107498f85e9765e3d upstream.

gcc 7 complains:
drivers/net/wireless/intersil/p54/fwio.c: In function 'p54_scan':
drivers/net/wireless/intersil/p54/fwio.c:491:4: warning: 'memset' used with length equal to number of elements without multiplication by element size [-Wmemset-elt-size]

Fix that by passing the correct size to memset.

---
 drivers/net/wireless/intersil/p54/fwio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/wireless/intersil/p54/fwio.c int p54_scan(struct p54_common *priv, u1 · line 494

Quoted 8 lines
drivers/net/wireless/intersil/p54/fwio.c +1 -1
@@ -488,7 +488,7 @@ int p54_scan(struct p54_common *priv, u1
 
 			entry += sizeof(__le16);
 			chan->pa_points_per_curve = 8;
-			memset(chan->curve_data, 0, sizeof(*chan->curve_data));
+			memset(chan->curve_data, 0, sizeof(chan->curve_data));
 			memcpy(chan->curve_data, entry,
 			       sizeof(struct p54_pa_curve_data_sample) *
 			       min((u8)8, curve_data->points_per_channel));

Why is this change correct?

curve_data is a pointer.

This now clears the sizeof a pointer and not
the sizeof struct p54_cal_database

#3 lore raw

Re: [PATCH 4.9 01/12] p54: memset(0) whole array

Quoted 41 lines · Joe Perches
Quoted 29 lines · Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Jiri Slaby <[email protected]>

commit 6f17581788206444cbbcdbc107498f85e9765e3d upstream.

gcc 7 complains:
drivers/net/wireless/intersil/p54/fwio.c: In function 'p54_scan':
drivers/net/wireless/intersil/p54/fwio.c:491:4: warning: 'memset' used with length equal to number of elements without multiplication by element size [-Wmemset-elt-size]

Fix that by passing the correct size to memset.

---
 drivers/net/wireless/intersil/p54/fwio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/wireless/intersil/p54/fwio.c int p54_scan(struct p54_common *priv, u1 · line 494

Quoted 8 lines
drivers/net/wireless/intersil/p54/fwio.c +1 -1
@@ -488,7 +488,7 @@ int p54_scan(struct p54_common *priv, u1
 
 			entry += sizeof(__le16);
 			chan->pa_points_per_curve = 8;
-			memset(chan->curve_data, 0, sizeof(*chan->curve_data));
+			memset(chan->curve_data, 0, sizeof(chan->curve_data));
 			memcpy(chan->curve_data, entry,
 			       sizeof(struct p54_pa_curve_data_sample) *
 			       min((u8)8, curve_data->points_per_channel));

Why is this change correct?

Quoted 4 lines

curve_data is a pointer.

This now clears the sizeof a pointer and not
the sizeof struct p54_cal_database

So what happens here?
This change seems clearly incorrect.
For all stable versions.

#4 lore raw

Re: [PATCH 4.9 01/12] p54: memset(0) whole array

Quoted 42 lines · CEST Joe Perches
Quoted 41 lines · Joe Perches
Quoted 29 lines · Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Jiri Slaby <[email protected]>

commit 6f17581788206444cbbcdbc107498f85e9765e3d upstream.

gcc 7 complains:
drivers/net/wireless/intersil/p54/fwio.c: In function 'p54_scan':
drivers/net/wireless/intersil/p54/fwio.c:491:4: warning: 'memset' used with length equal to number of elements without multiplication by element size [-Wmemset-elt-size]

Fix that by passing the correct size to memset.

---
 drivers/net/wireless/intersil/p54/fwio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/wireless/intersil/p54/fwio.c int p54_scan(struct p54_common *priv, u1 · line 494

Quoted 8 lines
drivers/net/wireless/intersil/p54/fwio.c +1 -1
@@ -488,7 +488,7 @@ int p54_scan(struct p54_common *priv, u1
 
 			entry += sizeof(__le16);
 			chan->pa_points_per_curve = 8;
-			memset(chan->curve_data, 0, sizeof(*chan->curve_data));
+			memset(chan->curve_data, 0, sizeof(chan->curve_data));
 			memcpy(chan->curve_data, entry,
 			       sizeof(struct p54_pa_curve_data_sample) *
 			       min((u8)8, curve_data->points_per_channel));

Why is this change correct?

Quoted 8 lines
Quoted 4 lines

curve_data is a pointer.

This now clears the sizeof a pointer and not
the sizeof struct p54_cal_database

So what happens here?
This change seems clearly incorrect.
For all stable versions.

hm?

Please, just look again at [0]:

|	struct p54_scan_body *chan = &body->normal;
|	struct pda_pa_curve_data *curve_data =
|				(void *) priv->curve_data->data;
|
|	entry += sizeof(__le16);
|	chan->pa_points_per_curve = 8;
|	memset(chan->curve_data, 0, sizeof(chan->curve_data));
|	memcpy(chan->curve_data, entry,
|	       sizeof(struct p54_pa_curve_data_sample) *
|	       min((u8)8, curve_data->points_per_channel));

yes: "curve_data" is a pointer. But memset and memcpy are using "chan->curve_data". chan is a pointer to a p54_scan_body struct. The structure is defined in lmac.h:

|struct p54_pa_curve_data_sample {
|	u8 rf_power;
|	u8 pa_detector;
|	u8 data_barker;
|	u8 data_bpsk;
|	u8 data_qpsk;
|	u8 data_16qam;
|	u8 data_64qam;
|	u8 padding;
|} __packed;
|
|struct p54_scan_body {
|	u8 pa_points_per_curve;
|	u8 val_barker;
|	u8 val_bpsk;
|	u8 val_qpsk;
|	u8 val_16qam;
|	u8 val_64qam;
|	struct p54_pa_curve_data_sample curve_data[8];
|	u8 dup_bpsk;
|	u8 dup_qpsk;
|	u8 dup_16qam;
|	u8 dup_64qam;
|} __packed;

p54_scan_body's curve_data is an array of eight p54_pa_curve_data_sample, each with eight u8. This means that chan->curve_data is 64 bytes in total.

If you are not convinced yet, please add:

BUILD_BUG_ON(sizeof(chan->curve_data) != 64);

next to the memset and compile the driver.
If this was all wrong, this would cause a build error, right?

Regards,
Christian

[0] <http://elixir.free-electrons.com/linux/v4.13-rc7/source/drivers/net/wireless/intersil/p54/fwio.c#L485>
[1] <http://elixir.free-electrons.com/linux/v4.13-rc7/source/drivers/net/wireless/intersil/p54/lmac.h#L351>

#5 lore raw

Re: [PATCH 4.9 01/12] p54: memset(0) whole array

Quoted 4 lines · Christian Lamparter
Quoted 3 lines · CEST Joe Perches
Quoted 2 lines · Joe Perches
Quoted 1 line · Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

[]

Quoted 19 lines
Quoted 19 lines
Quoted 18 lines

drivers/net/wireless/intersil/p54/fwio.c int p54_scan(struct p54_common *priv, u1 · line 494

Quoted 11 lines

+++ b/drivers/net/wireless/intersil/p54/fwio.c

drivers/net/wireless/intersil/p54/fwio.c +1 -1
@@ -488,7 +488,7 @@ int p54_scan(struct p54_common *priv, u1
 
 			entry += sizeof(__le16);
 			chan->pa_points_per_curve = 8;
-			memset(chan->curve_data, 0, sizeof(*chan->curve_data));
+			memset(chan->curve_data, 0, sizeof(chan->curve_data));
 			memcpy(chan->curve_data, entry,
 			       sizeof(struct p54_pa_curve_data_sample) *
 			       min((u8)8, curve_data->points_per_channel));

Why is this change correct?

curve_data is a pointer.

This now clears the sizeof a pointer and not
the sizeof struct p54_cal_database

Quoted 7 lines
Quoted 3 lines

So what happens here?
This change seems clearly incorrect.
For all stable versions.

hm?

Please, just look again at [0]:

[]

Quoted 1 line

yes: "curve_data" is a pointer.

Thank you.
My mistake, curve_data here is an array of structs.

Quoted 1 line

This means that chan->curve_data is 64 bytes in total.

I saw only:

drivers/net/wireless/intersil/p54/p54.h:��������struct p54_cal_database *curve_data;

and did not notice

drivers/net/wireless/intersil/p54/lmac.h: struct p54_pa_curve_data_sample curve_data[8];

author maintainer
#6 lore raw

PATCH 2/12 scsi: isci: avoid array subscript warning

In reply to Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Arnd Bergmann <[email protected]>

commit 5cfa2a3c7342bd0b50716c8bb32ee491af43c785 upstream.

I'm getting a new warning with gcc-7:

isci/remote_node_context.c: In function 'sci_remote_node_context_destruct':
isci/remote_node_context.c:69:16: error: array subscript is above array bounds [-Werror=array-bounds]

This is odd, since we clearly cover all values for enum scis_sds_remote_node_context_states here. Anyway, checking for an array overflow can't harm and it makes the warning go away.

---
 drivers/scsi/isci/remote_node_context.c |    3 +++
 1 file changed, 3 insertions(+)
drivers/scsi/isci/remote_node_context.c +3 -0
@@ -66,6 +66,9 @@ const char *rnc_state_name(enum scis_sds
 {
 	static const char * const strings[] = RNC_STATES;
 
+	if (state >= ARRAY_SIZE(strings))
+		return "UNKNOWN";
+
 	return strings[state];
 }
 #undef C
author maintainer
#7 lore raw

PATCH 3/12 staging: wilc1000: simplify vif[i]->ndev accesses

In reply to Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Arnd Bergmann <[email protected]>

commit 735bb39ca3bed8469b3b3a42d8cc57bdb9fc4dd7 upstream.

With gcc-7, I got a new warning for this driver:

wilc1000/linux_wlan.c: In function 'wilc_netdev_cleanup':
wilc1000/linux_wlan.c:1224:15: error: 'vif[1]' may be used uninitialized in this function [-Werror=maybe-uninitialized] wilc1000/linux_wlan.c:1224:15: error: 'vif[0]' may be used uninitialized in this function [-Werror=maybe-uninitialized]

A closer look at the function reveals that it's more complex than it needs to be, given that based on how the device is created we always get

	netdev_priv(vif->ndev) == vif

Based on this assumption, I found a few other places in the same file that can be simplified. That code appears to be a relic from times when the assumption above was not valid.

---
 drivers/staging/wilc1000/linux_wlan.c |   34 +++++++++-------------------------
 1 file changed, 9 insertions(+), 25 deletions(-)
drivers/staging/wilc1000/linux_wlan.c +9 -25
@@ -269,23 +269,12 @@ static struct net_device *get_if_handler
 
 int wilc_wlan_set_bssid(struct net_device *wilc_netdev, u8 *bssid, u8 mode)
 {
-	int i = 0;
-	int ret = -1;
-	struct wilc_vif *vif;
-	struct wilc *wilc;
-
-	vif = netdev_priv(wilc_netdev);
-	wilc = vif->wilc;
+	struct wilc_vif *vif = netdev_priv(wilc_netdev);
 
-	for (i = 0; i < wilc->vif_num; i++)
-		if (wilc->vif[i]->ndev == wilc_netdev) {
-			memcpy(wilc->vif[i]->bssid, bssid, 6);
-			wilc->vif[i]->mode = mode;
-			ret = 0;
-			break;
-		}
+	memcpy(vif->bssid, bssid, 6);
+	vif->mode = mode;
 
-	return ret;
+	return 0;
 }
 
 int wilc_wlan_get_num_conn_ifcs(struct wilc *wilc)
@@ -1212,16 +1201,11 @@ void WILC_WFI_mgmt_rx(struct wilc *wilc,
 
 void wilc_netdev_cleanup(struct wilc *wilc)
 {
-	int i = 0;
-	struct wilc_vif *vif[NUM_CONCURRENT_IFC];
+	int i;
 
-	if (wilc && (wilc->vif[0]->ndev || wilc->vif[1]->ndev)) {
+	if (wilc && (wilc->vif[0]->ndev || wilc->vif[1]->ndev))
 		unregister_inetaddr_notifier(&g_dev_notifier);
 
-		for (i = 0; i < NUM_CONCURRENT_IFC; i++)
-			vif[i] = netdev_priv(wilc->vif[i]->ndev);
-	}
-
 	if (wilc && wilc->firmware) {
 		release_firmware(wilc->firmware);
 		wilc->firmware = NULL;
@@ -1230,7 +1214,7 @@ void wilc_netdev_cleanup(struct wilc *wi
 	if (wilc && (wilc->vif[0]->ndev || wilc->vif[1]->ndev)) {
 		for (i = 0; i < NUM_CONCURRENT_IFC; i++)
 			if (wilc->vif[i]->ndev)
-				if (vif[i]->mac_opened)
+				if (wilc->vif[i]->mac_opened)
 					wilc_mac_close(wilc->vif[i]->ndev);
 
 		for (i = 0; i < NUM_CONCURRENT_IFC; i++) {
@@ -1278,9 +1262,9 @@ int wilc_netdev_init(struct wilc **wilc,
 
 		vif->idx = wl->vif_num;
 		vif->wilc = *wilc;
+		vif->ndev = ndev;
 		wl->vif[i] = vif;
-		wl->vif[wl->vif_num]->ndev = ndev;
-		wl->vif_num++;
+		wl->vif_num = i;
 		ndev->netdev_ops = &wilc_netdev_ops;
 
 		{
author maintainer
#8 lore raw

PATCH 4/12 gcov: support GCC 7.1

In reply to Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Martin Liska <[email protected]>

commit 05384213436ab690c46d9dfec706b80ef8d671ab upstream.

Starting from GCC 7.1, __gcov_exit is a new symbol expected to be implemented in a profiling runtime.

---
 kernel/gcov/base.c    |    6 ++++++
 kernel/gcov/gcc_4_7.c |    4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)
kernel/gcov/base.c +6 -0
@@ -98,6 +98,12 @@ void __gcov_merge_icall_topn(gcov_type *
 }
 EXPORT_SYMBOL(__gcov_merge_icall_topn);
 
+void __gcov_exit(void)
+{
+	/* Unused. */
+}
+EXPORT_SYMBOL(__gcov_exit);
+
 /**
  * gcov_enable_events - enable event reporting through gcov_event()
  *
kernel/gcov/gcc_4_7.c +3 -1
@@ -18,7 +18,9 @@
 #include <linux/vmalloc.h>
 #include "gcov.h"
 
-#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
+#if (__GNUC__ >= 7)
+#define GCOV_COUNTERS			9
+#elif (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
 #define GCOV_COUNTERS			10
 #elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
 #define GCOV_COUNTERS			9
author maintainer
#9 lore raw

PATCH 6/12 arm64: mm: abort uaccess retries upon fatal signal

In reply to Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Mark Rutland <[email protected]>

commit 289d07a2dc6c6b6f3e4b8a62669320d99dbe6c3d upstream.

When there's a fatal signal pending, arm64's do_page_fault() implementation returns 0. The intent is that we'll return to the faulting userspace instruction, delivering the signal on the way.

However, if we take a fatal signal during fixing up a uaccess, this results in a return to the faulting kernel instruction, which will be instantly retried, resulting in the same fault being taken forever. As the task never reaches userspace, the signal is not delivered, and the task is left unkillable. While the task is stuck in this state, it can inhibit the forward progress of the system.

To avoid this, we must ensure that when a fatal signal is pending, we apply any necessary fixup for a faulting kernel instruction. Thus we will return to an error path, and it is up to that code to make forward progress towards delivering the fatal signal.

---
 arch/arm64/mm/fault.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
arch/arm64/mm/fault.c +4 -1
@@ -373,8 +373,11 @@ retry:
 	 * signal first. We do not need to release the mmap_sem because it
 	 * would already be released in __lock_page_or_retry in mm/filemap.c.
 	 */
-	if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current))
+	if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current)) {
+		if (!user_mode(regs))
+			goto no_context;
 		return 0;
+	}
 
 	/*
 	 * Major/minor page fault accounting is only done on the initial
author maintainer
#10 lore raw

PATCH 8/12 arm64: fpsimd: Prevent registers leaking across exec

In reply to Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Dave Martin <[email protected]>

commit 096622104e14d8a1db4860bd557717067a0515d2 upstream.

There are some tricky dependencies between the different stages of flushing the FPSIMD register state during exec, and these can race with context switch in ways that can cause the old task's regs to leak across. In particular, a context switch during the memset() can cause some of the task's old FPSIMD registers to reappear.

Disabling preemption for this small window would be no big deal for
performance: preemption is already disabled for similar scenarios like updating the FPSIMD registers in sigreturn.

So, instead of rearranging things in ways that might swap existing subtle bugs for new ones, this patch just disables preemption around the FPSIMD state flushing so that races of this type can't occur here. This brings fpsimd_flush_thread() into line with other code paths.

Fixes 674c242c9323 ("arm64: flush FP/SIMD state correctly after execve()")

---

For stable only.

3.17.x-4.0.x don't appear active, and this patch isn't sufficient to fix them (they would need 674c242c9323 also).

 arch/arm64/kernel/fpsimd.c |    2 ++
 1 file changed, 2 insertions(+)
arch/arm64/kernel/fpsimd.c +2 -0
@@ -157,9 +157,11 @@ void fpsimd_thread_switch(struct task_st
 
 void fpsimd_flush_thread(void)
 {
+	preempt_disable();
 	memset(&current->thread.fpsimd_state, 0, sizeof(struct fpsimd_state));
 	fpsimd_flush_task_state(current);
 	set_thread_flag(TIF_FOREIGN_FPSTATE);
+	preempt_enable();
 }
 
 /*
author maintainer
#11 lore raw

PATCH 9/12 locking/spinlock/debug: Remove spinlock lockup detection code

In reply to Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Waiman Long <[email protected]>

commit bc88c10d7e6900916f5e1ba3829d66a9de92b633 upstream.

The current spinlock lockup detection code can sometimes produce false positives because of the unfairness of the locking algorithm itself.

So the lockup detection code is now removed. Instead, we are relying on the NMI watchdog to detect potential lockup. We won't have lockup detection if the watchdog isn't running.

The commented-out read-write lock lockup detection code are also removed.

---
 kernel/locking/spinlock_debug.c |   86 ++--------------------------------------
 1 file changed, 5 insertions(+), 81 deletions(-)
kernel/locking/spinlock_debug.c +5 -81
@@ -103,38 +103,14 @@ static inline void debug_spin_unlock(raw
 	lock->owner_cpu = -1;
 }
 
-static void __spin_lock_debug(raw_spinlock_t *lock)
-{
-	u64 i;
-	u64 loops = loops_per_jiffy * HZ;
-
-	for (i = 0; i < loops; i++) {
-		if (arch_spin_trylock(&lock->raw_lock))
-			return;
-		__delay(1);
-	}
-	/* lockup suspected: */
-	spin_dump(lock, "lockup suspected");
-#ifdef CONFIG_SMP
-	trigger_all_cpu_backtrace();
-#endif
-
-	/*
-	 * The trylock above was causing a livelock.  Give the lower level arch
-	 * specific lock code a chance to acquire the lock. We have already
-	 * printed a warning/backtrace at this point. The non-debug arch
-	 * specific code might actually succeed in acquiring the lock.  If it is
-	 * not successful, the end-result is the same - there is no forward
-	 * progress.
-	 */
-	arch_spin_lock(&lock->raw_lock);
-}
-
+/*
+ * We are now relying on the NMI watchdog to detect lockup instead of doing
+ * the detection here with an unfair lock which can cause problem of its own.
+ */
 void do_raw_spin_lock(raw_spinlock_t *lock)
 {
 	debug_spin_lock_before(lock);
-	if (unlikely(!arch_spin_trylock(&lock->raw_lock)))
-		__spin_lock_debug(lock);
+	arch_spin_lock(&lock->raw_lock);
 	debug_spin_lock_after(lock);
 }
 
@@ -172,32 +148,6 @@ static void rwlock_bug(rwlock_t *lock, c
 
 #define RWLOCK_BUG_ON(cond, lock, msg) if (unlikely(cond)) rwlock_bug(lock, msg)
 
-#if 0		/* __write_lock_debug() can lock up - maybe this can too? */
-static void __read_lock_debug(rwlock_t *lock)
-{
-	u64 i;
-	u64 loops = loops_per_jiffy * HZ;
-	int print_once = 1;
-
-	for (;;) {
-		for (i = 0; i < loops; i++) {
-			if (arch_read_trylock(&lock->raw_lock))
-				return;
-			__delay(1);
-		}
-		/* lockup suspected: */
-		if (print_once) {
-			print_once = 0;
-			printk(KERN_EMERG "BUG: read-lock lockup on CPU#%d, "
-					"%s/%d, %p\n",
-				raw_smp_processor_id(), current->comm,
-				current->pid, lock);
-			dump_stack();
-		}
-	}
-}
-#endif
-
 void do_raw_read_lock(rwlock_t *lock)
 {
 	RWLOCK_BUG_ON(lock->magic != RWLOCK_MAGIC, lock, "bad magic");
@@ -247,32 +197,6 @@ static inline void debug_write_unlock(rw
 	lock->owner_cpu = -1;
 }
 
-#if 0		/* This can cause lockups */
-static void __write_lock_debug(rwlock_t *lock)
-{
-	u64 i;
-	u64 loops = loops_per_jiffy * HZ;
-	int print_once = 1;
-
-	for (;;) {
-		for (i = 0; i < loops; i++) {
-			if (arch_write_trylock(&lock->raw_lock))
-				return;
-			__delay(1);
-		}
-		/* lockup suspected: */
-		if (print_once) {
-			print_once = 0;
-			printk(KERN_EMERG "BUG: write-lock lockup on CPU#%d, "
-					"%s/%d, %p\n",
-				raw_smp_processor_id(), current->comm,
-				current->pid, lock);
-			dump_stack();
-		}
-	}
-}
-#endif
-
 void do_raw_write_lock(rwlock_t *lock)
 {
 	debug_write_lock_before(lock);
author maintainer
#12 lore raw

PATCH 10/12 scsi: sg: protect accesses to reserved page array

In reply to Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Hannes Reinecke <[email protected]>

commit 1bc0eb0446158cc76562176b80623aa119afee5b upstream.

The 'reserved' page array is used as a short-cut for mapping data, saving us to allocate pages per request. However, the 'reserved' array is only capable of holding one request, so this patch introduces a mutex for protect 'sg_fd' against concurrent accesses.

[[email protected]: backport to 3.18-4.9, fixup for bad ioctl SG_SET_FORCE_LOW_DMA code removed in later versions and not modified by the original patch.]

---
 drivers/scsi/sg.c |   47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)
drivers/scsi/sg.c +26 -21
@@ -142,6 +142,7 @@ typedef struct sg_fd {		/* holds the sta
 	struct sg_device *parentdp;	/* owning device */
 	wait_queue_head_t read_wait;	/* queue read until command done */
 	rwlock_t rq_list_lock;	/* protect access to list in req_arr */
+	struct mutex f_mutex;	/* protect against changes in this fd */
 	int timeout;		/* defaults to SG_DEFAULT_TIMEOUT      */
 	int timeout_user;	/* defaults to SG_DEFAULT_TIMEOUT_USER */
 	Sg_scatter_hold reserve;	/* buffer held for this file descriptor */
@@ -155,6 +156,7 @@ typedef struct sg_fd {		/* holds the sta
 	unsigned char next_cmd_len; /* 0: automatic, >0: use on next write() */
 	char keep_orphan;	/* 0 -> drop orphan (def), 1 -> keep for read() */
 	char mmap_called;	/* 0 -> mmap() never called on this fd */
+	char res_in_use;	/* 1 -> 'reserve' array in use */
 	struct kref f_ref;
 	struct execute_work ew;
 } Sg_fd;
@@ -198,7 +200,6 @@ static void sg_remove_sfp(struct kref *)
 static Sg_request *sg_get_rq_mark(Sg_fd * sfp, int pack_id);
 static Sg_request *sg_add_request(Sg_fd * sfp);
 static int sg_remove_request(Sg_fd * sfp, Sg_request * srp);
-static int sg_res_in_use(Sg_fd * sfp);
 static Sg_device *sg_get_dev(int dev);
 static void sg_device_destroy(struct kref *kref);
 
@@ -614,6 +615,7 @@ sg_write(struct file *filp, const char _
 	}
 	buf += SZ_SG_HEADER;
 	__get_user(opcode, buf);
+	mutex_lock(&sfp->f_mutex);
 	if (sfp->next_cmd_len > 0) {
 		cmd_size = sfp->next_cmd_len;
 		sfp->next_cmd_len = 0;	/* reset so only this write() effected */
@@ -622,6 +624,7 @@ sg_write(struct file *filp, const char _
 		if ((opcode >= 0xc0) && old_hdr.twelve_byte)
 			cmd_size = 12;
 	}
+	mutex_unlock(&sfp->f_mutex);
 	SCSI_LOG_TIMEOUT(4, sg_printk(KERN_INFO, sdp,
 		"sg_write:   scsi opcode=0x%02x, cmd_size=%d\n", (int) opcode, cmd_size));
 /* Determine buffer size.  */
@@ -721,7 +724,7 @@ sg_new_write(Sg_fd *sfp, struct file *fi
 			sg_remove_request(sfp, srp);
 			return -EINVAL;	/* either MMAP_IO or DIRECT_IO (not both) */
 		}
-		if (sg_res_in_use(sfp)) {
+		if (sfp->res_in_use) {
 			sg_remove_request(sfp, srp);
 			return -EBUSY;	/* reserve buffer already being used */
 		}
@@ -892,7 +895,7 @@ sg_ioctl(struct file *filp, unsigned int
 			return result;
 		if (val) {
 			sfp->low_dma = 1;
-			if ((0 == sfp->low_dma) && (0 == sg_res_in_use(sfp))) {
+			if ((0 == sfp->low_dma) && !sfp->res_in_use) {
 				val = (int) sfp->reserve.bufflen;
 				sg_remove_scat(sfp, &sfp->reserve);
 				sg_build_reserve(sfp, val);
@@ -967,12 +970,18 @@ sg_ioctl(struct file *filp, unsigned int
                         return -EINVAL;
 		val = min_t(int, val,
 			    max_sectors_bytes(sdp->device->request_queue));
+		mutex_lock(&sfp->f_mutex);
 		if (val != sfp->reserve.bufflen) {
-			if (sg_res_in_use(sfp) || sfp->mmap_called)
+			if (sfp->mmap_called ||
+			    sfp->res_in_use) {
+				mutex_unlock(&sfp->f_mutex);
 				return -EBUSY;
+			}
+
 			sg_remove_scat(sfp, &sfp->reserve);
 			sg_build_reserve(sfp, val);
 		}
+		mutex_unlock(&sfp->f_mutex);
 		return 0;
 	case SG_GET_RESERVED_SIZE:
 		val = min_t(int, sfp->reserve.bufflen,
@@ -1727,13 +1736,22 @@ sg_start_req(Sg_request *srp, unsigned c
 		md = &map_data;
 
 	if (md) {
-		if (!sg_res_in_use(sfp) && dxfer_len <= rsv_schp->bufflen)
+		mutex_lock(&sfp->f_mutex);
+		if (dxfer_len <= rsv_schp->bufflen &&
+		    !sfp->res_in_use) {
+			sfp->res_in_use = 1;
 			sg_link_reserve(sfp, srp, dxfer_len);
-		else {
+		} else if ((hp->flags & SG_FLAG_MMAP_IO) && sfp->res_in_use) {
+			mutex_unlock(&sfp->f_mutex);
+			return -EBUSY;
+		} else {
 			res = sg_build_indirect(req_schp, sfp, dxfer_len);
-			if (res)
+			if (res) {
+				mutex_unlock(&sfp->f_mutex);
 				return res;
+			}
 		}
+		mutex_unlock(&sfp->f_mutex);
 
 		md->pages = req_schp->pages;
 		md->page_order = req_schp->page_order;
@@ -2135,6 +2153,7 @@ sg_add_sfp(Sg_device * sdp)
 	rwlock_init(&sfp->rq_list_lock);
 
 	kref_init(&sfp->f_ref);
+	mutex_init(&sfp->f_mutex);
 	sfp->timeout = SG_DEFAULT_TIMEOUT;
 	sfp->timeout_user = SG_DEFAULT_TIMEOUT_USER;
 	sfp->force_packid = SG_DEF_FORCE_PACK_ID;
@@ -2210,20 +2229,6 @@ sg_remove_sfp(struct kref *kref)
 	schedule_work(&sfp->ew.work);
 }
 
-static int
-sg_res_in_use(Sg_fd * sfp)
-{
-	const Sg_request *srp;
-	unsigned long iflags;
-
-	read_lock_irqsave(&sfp->rq_list_lock, iflags);
-	for (srp = sfp->headrp; srp; srp = srp->nextrp)
-		if (srp->res_used)
-			break;
-	read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
-	return srp ? 1 : 0;
-}
-
 #ifdef CONFIG_SCSI_PROC_FS
 static int
 sg_idr_max_id(int id, void *p, void *data)
author maintainer
#13 lore raw

PATCH 11/12 scsi: sg: reset res_in_use after unlinking reserved array

In reply to Greg Kroah-Hartman

4.9-stable review patch. If anyone has any objections, please let me know.

------------------

From: Hannes Reinecke <[email protected]>

commit e791ce27c3f6a1d3c746fd6a8f8e36c9540ec6f9 upstream.

Once the reserved page array is unused we can reset the 'res_in_use' state; here we can do a lazy update without holding the mutex as we only need to check against concurrent access, not concurrent release.

[mkp: checkpatch]

Fixes 1bc0eb044615 ("scsi: sg: protect accesses to 'reserved' page array")
Cc Todd Poynor <[email protected]>
---
 drivers/scsi/sg.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/scsi/sg.c +2 -0
@@ -2042,6 +2042,8 @@ sg_unlink_reserve(Sg_fd * sfp, Sg_reques
 	req_schp->sglist_len = 0;
 	sfp->save_scat_len = 0;
 	srp->res_used = 0;
+	/* Called without mutex lock to avoid deadlock */
+	sfp->res_in_use = 0;
 }
 
 static Sg_request *
#14 lore raw
In reply to Greg Kroah-Hartman

Hi Greg,

Quoted 7 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 4.9.47 release. There are 12 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Sat Sep 2 15:42:31 UTC 2017. Anything received after that time might be too late.

For ARM64, built with defconfig and booted on Hikey - no regressions noted.

Quoted 11 lines

The whole patch series can be found in one patch at:

        kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.47-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-4.9.y
and the diffstat can be found below.

thanks,

greg k-h

Best,
Sumit.

author maintainer
#15 lore raw
Quoted 9 lines · Sumit Semwal

Hi Greg,

Quoted 5 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 4.9.47 release. There are 12 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Quoted 4 lines
Quoted 2 lines

Responses should be made by Sat Sep 2 15:42:31 UTC 2017. Anything received after that time might be too late.

For ARM64, built with defconfig and booted on Hikey - no regressions noted.

thanks for testing and letting me know.

greg k-h

#16 lore raw
In reply to Greg Kroah-Hartman
Quoted 9 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 4.9.47 release. There are 12 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Sat Sep 2 15:42:31 UTC 2017. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

Quoted 9 lines
	kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.47-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-4.9.y
and the diffstat can be found below.

thanks,

greg k-h

Compiled and booted on my test system. No dmesg regressions.

thanks,
-- Shuah

author maintainer
#17 lore raw
Quoted 15 lines · Shuah Khan
Quoted 14 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 4.9.47 release. There are 12 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Sat Sep 2 15:42:31 UTC 2017. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

	kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.47-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-4.9.y
and the diffstat can be found below.

Quoted 6 lines
Quoted 4 lines

thanks,

greg k-h

Compiled and booted on my test system. No dmesg regressions.

Thanks for testing all of these and letting me know.

greg k-h

#18 lore raw
In reply to Greg Kroah-Hartman
Quoted 8 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 4.9.47 release. There are 12 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Sat Sep 2 15:42:31 UTC 2017. Anything received after that time might be too late.

Build results:

	total: 145 pass: 145 fail: 0
Qemu test results:
	total: 122 pass: 122 fail: 0

Details are available at http://kerneltests.org/builders.

Guenter