[PATCH v3 0/9] stm/intel_th: Updates for 4.4

[PATCH v3 0/9] stm/intel_th: Updates for 4.4

[Alexander Shishkin]

Hi Greg,

Since the previous versions ([1], [2]) didn't land anywhere yet, I'm
resending the whole thing with a few additions: Sasha reported a
user-controllable allocation with a possible overflow, Chunyan found
an off-by-one in another allocation path and I also found a couple of
locking issues that are better resolved sooner than later. Please
consider this for 4.4 still if possible.

[1] http://marc.info/?l=linux-kernel&m=144845503811729
[2] http://marc.info/?l=linux-kernel&m=144706387705997

Alexander Shishkin (5):
  stm class: Fix locking in unbinding policy path
  stm class: Fix link list locking
  stm class: Prevent user-controllable allocations
  intel_th: pci: Add Apollo Lake SOC support
  intel_th: pci: Add Broxton SOC support

Arnd Bergmann (1):
  stm class: Select CONFIG_SRCU

Chunyan Zhang (1):
  stm class: Fix an off-by-one in master array allocation

Geert Uytterhoeven (2):
  stm class: Hide STM-specific options if STM is disabled
  intel_th: INTEL_TH should depend on HAS_DMA

 drivers/hwtracing/intel_th/Kconfig |  1 +
 drivers/hwtracing/intel_th/pci.c   | 10 +++++++++
 drivers/hwtracing/stm/Kconfig      |  5 +++++
 drivers/hwtracing/stm/core.c       | 43 +++++++++++++++++++++++++++++---------
 drivers/hwtracing/stm/policy.c     | 18 +++++++++++++---
 drivers/hwtracing/stm/stm.h        |  1 +
 6 files changed, 65 insertions(+), 13 deletions(-)

-- 
2.6.4

[PATCH v3 1/9] stm class: Hide STM-specific options if STM is disabled

[Alexander Shishkin]

From: Geert Uytterhoeven <geert@linux-m68k.org>

If STM=n, it doesn't make sense to ask about STM_DUMMY and
STM_SOURCE_CONSOLE support, which are not even built when enabled
anyway. Hence hide these options if STM=n.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
index 83e9f591a5..4c13762f2b 100644
--- a/drivers/hwtracing/stm/Kconfig
+++ b/drivers/hwtracing/stm/Kconfig
@@ -8,6 +8,8 @@ config STM
 
 	  Say Y here to enable System Trace Module device support.
 
+if STM
+
 config STM_DUMMY
 	tristate "Dummy STM driver"
 	help
@@ -24,3 +26,5 @@ config STM_SOURCE_CONSOLE
 
 	  If you want to send kernel console messages over STM devices,
 	  say Y.
+
+endif
-- 
2.6.4

[PATCH v3 2/9] intel_th: INTEL_TH should depend on HAS_DMA

[Alexander Shishkin]

From: Geert Uytterhoeven <geert@linux-m68k.org>

If NO_DMA=y:

    ERROR: "dma_free_coherent" [drivers/hwtracing/intel_th/intel_th_msu.ko] undefined!
    ERROR: "dma_alloc_coherent" [drivers/hwtracing/intel_th/intel_th_msu.ko] undefined!
    ERROR: "dma_supported" [drivers/hwtracing/intel_th/intel_th.ko] undefined!

Add a dependency on HAS_DMA to fix this.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hwtracing/intel_th/Kconfig b/drivers/hwtracing/intel_th/Kconfig
index b7a9073d96..90b0844093 100644
--- a/drivers/hwtracing/intel_th/Kconfig
+++ b/drivers/hwtracing/intel_th/Kconfig
@@ -1,5 +1,6 @@
 config INTEL_TH
 	tristate "Intel(R) Trace Hub controller"
+	depends on HAS_DMA
 	help
 	  Intel(R) Trace Hub (TH) is a set of hardware blocks (subdevices) that
 	  produce, switch and output trace data from multiple hardware and
-- 
2.6.4

[PATCH v3 3/9] stm class: Select CONFIG_SRCU

[Alexander Shishkin]

From: Arnd Bergmann <arnd@arndb.de>

The newly added STM code uses SRCU, but does not ensure that
this code is part of the kernel:

drivers/built-in.o: In function `stm_source_link_show':
include/linux/srcu.h:221: undefined reference to `__srcu_read_lock'
include/linux/srcu.h:238: undefined reference to `__srcu_read_unlock'
drivers/built-in.o: In function `stm_source_link_drop':
include/linux/srcu.h:221: undefined reference to `__srcu_read_lock'
include/linux/srcu.h:238: undefined reference to `__srcu_read_unlock'

This adds a Kconfig 'select' statement like all the other SRCU using
drivers have.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 7bd1d4093c2f ("stm class: Introduce an abstraction for System Trace Module devices")
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
index 4c13762f2b..e0ac753955 100644
--- a/drivers/hwtracing/stm/Kconfig
+++ b/drivers/hwtracing/stm/Kconfig
@@ -1,6 +1,7 @@
 config STM
 	tristate "System Trace Module devices"
 	select CONFIGFS_FS
+	select SRCU
 	help
 	  A System Trace Module (STM) is a device exporting data in System
 	  Trace Protocol (STP) format as defined by MIPI STP standards.
-- 
2.6.4

[PATCH v3 4/9] stm class: Fix locking in unbinding policy path

[Alexander Shishkin]

Right now, if stm device removal has to unbind from a policy (that is,
an stm device that has STP policy, gets removed), it will trigger a
nested lock on the stm device's policy mutex.

This patch fixes the problem by moving the locking from the policy
unbinding to policy removal (configfs path), where it's actually needed;
the other caller of the policy unbinding function already takes the
mutex around the call.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/policy.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c
index 11ab6d01ad..94d3abfb73 100644
--- a/drivers/hwtracing/stm/policy.c
+++ b/drivers/hwtracing/stm/policy.c
@@ -272,13 +272,17 @@ void stp_policy_unbind(struct stp_policy *policy)
 {
 	struct stm_device *stm = policy->stm;
 
+	/*
+	 * stp_policy_release() will not call here if the policy is already
+	 * unbound; other users should not either, as no link exists between
+	 * this policy and anything else in that case
+	 */
 	if (WARN_ON_ONCE(!policy->stm))
 		return;
 
-	mutex_lock(&stm->policy_mutex);
-	stm->policy = NULL;
-	mutex_unlock(&stm->policy_mutex);
+	lockdep_assert_held(&stm->policy_mutex);
 
+	stm->policy = NULL;
 	policy->stm = NULL;
 
 	stm_put_device(stm);
@@ -287,8 +291,16 @@ void stp_policy_unbind(struct stp_policy *policy)
 static void stp_policy_release(struct config_item *item)
 {
 	struct stp_policy *policy = to_stp_policy(item);
+	struct stm_device *stm = policy->stm;
 
+	/* a policy *can* be unbound and still exist in configfs tree */
+	if (!stm)
+		return;
+
+	mutex_lock(&stm->policy_mutex);
 	stp_policy_unbind(policy);
+	mutex_unlock(&stm->policy_mutex);
+
 	kfree(policy);
 }
 
-- 
2.6.4

[PATCH v3 5/9] stm class: Fix link list locking

[Alexander Shishkin]

Currently, the list of stm_sources linked to an stm device is protected by
a spinlock, which also means that sources' .unlink() method is called under
this spinlock. However, this method may (and does) sleep, which means
trouble.

This patch slightly reworks locking around stm::link_list so that bits that
might_sleep() are called with a mutex held instead. Modification of this
list requires both mutex and spinlock to be held, while looking at the list
can be done under either mutex or spinlock.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/core.c | 38 +++++++++++++++++++++++++++++---------
 drivers/hwtracing/stm/stm.h  |  1 +
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index b6445d9e54..ddcb606ace 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -641,6 +641,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
 	if (err)
 		goto err_device;
 
+	mutex_init(&stm->link_mutex);
 	spin_lock_init(&stm->link_lock);
 	INIT_LIST_HEAD(&stm->link_list);
 
@@ -671,11 +672,11 @@ void stm_unregister_device(struct stm_data *stm_data)
 	struct stm_source_device *src, *iter;
 	int i;
 
-	spin_lock(&stm->link_lock);
+	mutex_lock(&stm->link_mutex);
 	list_for_each_entry_safe(src, iter, &stm->link_list, link_entry) {
 		__stm_source_link_drop(src, stm);
 	}
-	spin_unlock(&stm->link_lock);
+	mutex_unlock(&stm->link_mutex);
 
 	synchronize_srcu(&stm_source_srcu);
 
@@ -694,6 +695,17 @@ void stm_unregister_device(struct stm_data *stm_data)
 }
 EXPORT_SYMBOL_GPL(stm_unregister_device);
 
+/*
+ * stm::link_list access serialization uses a spinlock and a mutex; holding
+ * either of them guarantees that the list is stable; modification requires
+ * holding both of them.
+ *
+ * Lock ordering is as follows:
+ *   stm::link_mutex
+ *     stm::link_lock
+ *       src::link_lock
+ */
+
 /**
  * stm_source_link_add() - connect an stm_source device to an stm device
  * @src:	stm_source device
@@ -710,6 +722,7 @@ static int stm_source_link_add(struct stm_source_device *src,
 	char *id;
 	int err;
 
+	mutex_lock(&stm->link_mutex);
 	spin_lock(&stm->link_lock);
 	spin_lock(&src->link_lock);
 
@@ -719,6 +732,7 @@ static int stm_source_link_add(struct stm_source_device *src,
 
 	spin_unlock(&src->link_lock);
 	spin_unlock(&stm->link_lock);
+	mutex_unlock(&stm->link_mutex);
 
 	id = kstrdup(src->data->name, GFP_KERNEL);
 	if (id) {
@@ -756,6 +770,7 @@ fail_free_output:
 	stm_put_device(stm);
 
 fail_detach:
+	mutex_lock(&stm->link_mutex);
 	spin_lock(&stm->link_lock);
 	spin_lock(&src->link_lock);
 
@@ -764,6 +779,7 @@ fail_detach:
 
 	spin_unlock(&src->link_lock);
 	spin_unlock(&stm->link_lock);
+	mutex_unlock(&stm->link_mutex);
 
 	return err;
 }
@@ -776,13 +792,20 @@ fail_detach:
  * If @stm is @src::link, disconnect them from one another and put the
  * reference on the @stm device.
  *
- * Caller must hold stm::link_lock.
+ * Caller must hold stm::link_mutex.
  */
 static void __stm_source_link_drop(struct stm_source_device *src,
 				   struct stm_device *stm)
 {
 	struct stm_device *link;
 
+	lockdep_assert_held(&stm->link_mutex);
+
+	if (src->data->unlink)
+		src->data->unlink(src->data);
+
+	/* for stm::link_list modification, we hold both mutex and spinlock */
+	spin_lock(&stm->link_lock);
 	spin_lock(&src->link_lock);
 	link = srcu_dereference_check(src->link, &stm_source_srcu, 1);
 	if (WARN_ON_ONCE(link != stm)) {
@@ -791,13 +814,13 @@ static void __stm_source_link_drop(struct stm_source_device *src,
 	}
 
 	stm_output_free(link, &src->output);
-	/* caller must hold stm::link_lock */
 	list_del_init(&src->link_entry);
 	/* matches stm_find_device() from stm_source_link_store() */
 	stm_put_device(link);
 	rcu_assign_pointer(src->link, NULL);
 
 	spin_unlock(&src->link_lock);
+	spin_unlock(&stm->link_lock);
 }
 
 /**
@@ -819,12 +842,9 @@ static void stm_source_link_drop(struct stm_source_device *src)
 	stm = srcu_dereference(src->link, &stm_source_srcu);
 
 	if (stm) {
-		if (src->data->unlink)
-			src->data->unlink(src->data);
-
-		spin_lock(&stm->link_lock);
+		mutex_lock(&stm->link_mutex);
 		__stm_source_link_drop(src, stm);
-		spin_unlock(&stm->link_lock);
+		mutex_unlock(&stm->link_mutex);
 	}
 
 	srcu_read_unlock(&stm_source_srcu, idx);
diff --git a/drivers/hwtracing/stm/stm.h b/drivers/hwtracing/stm/stm.h
index 95ece0292c..97ee022414 100644
--- a/drivers/hwtracing/stm/stm.h
+++ b/drivers/hwtracing/stm/stm.h
@@ -45,6 +45,7 @@ struct stm_device {
 	int			major;
 	unsigned int		sw_nmasters;
 	struct stm_data		*data;
+	struct mutex		link_mutex;
 	spinlock_t		link_lock;
 	struct list_head	link_list;
 	/* master allocation */
-- 
2.6.4

[PATCH v3 6/9] stm class: Fix an off-by-one in master array allocation

[Alexander Shishkin]

From: Chunyan Zhang <zhang.chunyan@linaro.org>

Since both sw_start and sw_end are master indices, the size of array
that holds them is sw_end - sw_start + 1, which the current code gets
wrong, allocating one item less than required.

This patch corrects the allocation size, avoiding potential slab
corruption.

Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
[alexander.shishkin@linux.intel.com: re-wrote the commit message]
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index ddcb606ace..40a8b79ab7 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -618,7 +618,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
 	if (!stm_data->packet || !stm_data->sw_nchannels)
 		return -EINVAL;
 
-	nmasters = stm_data->sw_end - stm_data->sw_start;
+	nmasters = stm_data->sw_end - stm_data->sw_start + 1;
 	stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
 	if (!stm)
 		return -ENOMEM;
-- 
2.6.4

[PATCH v3 7/9] stm class: Prevent user-controllable allocations

[Alexander Shishkin]

Currently, the character device write method allocates a temporary buffer
for user's data, but the user's data size is not sanitized and can cause
arbitrarily large allocations via kzalloc() or an integer overflow that
will then result in overwriting kernel memory.

This patch trims the input buffer size to avoid these issues.

Reported-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index 40a8b79ab7..aef8ddb244 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -406,6 +406,9 @@ static ssize_t stm_char_write(struct file *file, const char __user *buf,
 	char *kbuf;
 	int err;
 
+	if (count + 1 > PAGE_SIZE)
+		count = PAGE_SIZE - 1;
+
 	/*
 	 * if no m/c have been assigned to this writer up to this
 	 * point, use "default" policy entry
-- 
2.6.4

[PATCH v3 8/9] intel_th: pci: Add Apollo Lake SOC support

[Alexander Shishkin]

This adds Intel(R) Trace Hub PCI ID for Apollo Lake SOC.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwtracing/intel_th/pci.c b/drivers/hwtracing/intel_th/pci.c
index 641e879360..b5760730e1 100644
--- a/drivers/hwtracing/intel_th/pci.c
+++ b/drivers/hwtracing/intel_th/pci.c
@@ -67,6 +67,11 @@ static const struct pci_device_id intel_th_pci_id_table[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xa126),
 		.driver_data = (kernel_ulong_t)0,
 	},
+	{
+		/* Apollo Lake */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a8e),
+		.driver_data = (kernel_ulong_t)0,
+	},
 	{ 0 },
 };
 
-- 
2.6.4

[PATCH v3 9/9] intel_th: pci: Add Broxton SOC support

[Alexander Shishkin]

This adds Intel(R) Trace Hub PCI ID for Broxton SOC.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwtracing/intel_th/pci.c b/drivers/hwtracing/intel_th/pci.c
index b5760730e1..09017073d7 100644
--- a/drivers/hwtracing/intel_th/pci.c
+++ b/drivers/hwtracing/intel_th/pci.c
@@ -72,6 +72,11 @@ static const struct pci_device_id intel_th_pci_id_table[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a8e),
 		.driver_data = (kernel_ulong_t)0,
 	},
+	{
+		/* Broxton */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x0a80),
+		.driver_data = (kernel_ulong_t)0,
+	},
 	{ 0 },
 };
 
-- 
2.6.4