[PATCH 0/3] coresight: Don't use contextID with PID namespaces

[PATCH 0/3] coresight: Don't use contextID with PID namespaces

[Mathieu Poirier]

Since the in-kernel value of a PID differs from what is seen from 
PID namespaces, using contextID tracing with PID namespaces makes the
feature confusing to use and potentially subject to leaking out internal
kernel information.

This set returns an error if contextID and PID namespaces are used in
conjunction and gets rid of the vpid-to-pid translation function as it
is no longer needed.

Thanks,
Mathieu

Mathieu Poirier (3):
  coresight: etm3x: Don't use contextID with PID namespaces
  coresight: etm4x: Don't use contextID with PID namespaces
  coresight: Remove function coresight_vpid_to_pid()

 drivers/hwtracing/coresight/coresight-etm.h        |  3 --
 .../hwtracing/coresight/coresight-etm3x-sysfs.c    | 43 +++++++++++++++++---
 drivers/hwtracing/coresight/coresight-etm3x.c      |  4 +-
 .../hwtracing/coresight/coresight-etm4x-sysfs.c    | 47 +++++++++++++++++-----
 drivers/hwtracing/coresight/coresight-etm4x.h      |  3 --
 include/linux/coresight.h                          | 20 ---------
 6 files changed, 76 insertions(+), 44 deletions(-)

-- 
2.7.4

[PATCH 1/3] coresight: etm3x: Don't use contextID with PID namespaces

[Mathieu Poirier]

Tracers can trigger trace acquisition based on contextID value, something
that isn't useful when PID namespaces are enabled.  Indeed the PID value
of a process has a different representation in the kernel and the PID
namespace, making the feature confusing and potentially leaking internal
kernel information.

As such simply return an error when the feature is being used from a
PID namespace other than the default one.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm.h        |  3 --
 .../hwtracing/coresight/coresight-etm3x-sysfs.c    | 43 +++++++++++++++++++---
 drivers/hwtracing/coresight/coresight-etm3x.c      |  4 +-
 3 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
index e8b4549e30e2..79e1ad860d8a 100644
--- a/drivers/hwtracing/coresight/coresight-etm.h
+++ b/drivers/hwtracing/coresight/coresight-etm.h
@@ -168,8 +168,6 @@
  * @seq_curr_state: current value of the sequencer register.
  * @ctxid_idx: index for the context ID registers.
  * @ctxid_pid: value for the context ID to trigger on.
- * @ctxid_vpid:	Virtual PID seen by users if PID namespace is enabled, otherwise
- *		the same value of ctxid_pid.
  * @ctxid_mask: mask applicable to all the context IDs.
  * @sync_freq:	Synchronisation frequency.
  * @timestamp_event: Defines an event that requests the insertion
@@ -202,7 +200,6 @@ struct etm_config {
 	u32				seq_curr_state;
 	u8				ctxid_idx;
 	u32				ctxid_pid[ETM_MAX_CTXID_CMP];
-	u32				ctxid_vpid[ETM_MAX_CTXID_CMP];
 	u32				ctxid_mask;
 	u32				sync_freq;
 	u32				timestamp_event;
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index 9435c1481f61..75487b3fad86 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -4,6 +4,7 @@
  * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
  */
 
+#include <linux/pid_namespace.h>
 #include <linux/pm_runtime.h>
 #include <linux/sysfs.h>
 #include "coresight-etm.h"
@@ -1025,8 +1026,15 @@ static ssize_t ctxid_pid_show(struct device *dev,
 	struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct etm_config *config = &drvdata->config;
 
+	/*
+	 * Don't use contextID tracing if coming from a PID namespace.  See
+	 * comment in ctxid_pid_store().
+	 */
+	if (task_active_pid_ns(current) != &init_pid_ns)
+		return -EINVAL;
+
 	spin_lock(&drvdata->spinlock);
-	val = config->ctxid_vpid[config->ctxid_idx];
+	val = config->ctxid_pid[config->ctxid_idx];
 	spin_unlock(&drvdata->spinlock);
 
 	return sprintf(buf, "%#lx\n", val);
@@ -1037,19 +1045,28 @@ static ssize_t ctxid_pid_store(struct device *dev,
 			       const char *buf, size_t size)
 {
 	int ret;
-	unsigned long vpid, pid;
+	unsigned long pid;
 	struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct etm_config *config = &drvdata->config;
 
-	ret = kstrtoul(buf, 16, &vpid);
+	/*
+	 * When contextID tracing is enabled the tracers will insert the
+	 * value found in the contextID register in the trace stream.  But if
+	 * a process is in a namespace the PID of that process as seen from the
+	 * namespace won't be what the kernel sees, something that makes the
+	 * feature confusing and can potentially leak kernel only information.
+	 * As such refuse to use the feature if @current is not in the initial
+	 * PID namespace.
+	 */
+	if (task_active_pid_ns(current) != &init_pid_ns)
+		return -EINVAL;
+
+	ret = kstrtoul(buf, 16, &pid);
 	if (ret)
 		return ret;
 
-	pid = coresight_vpid_to_pid(vpid);
-
 	spin_lock(&drvdata->spinlock);
 	config->ctxid_pid[config->ctxid_idx] = pid;
-	config->ctxid_vpid[config->ctxid_idx] = vpid;
 	spin_unlock(&drvdata->spinlock);
 
 	return size;
@@ -1063,6 +1080,13 @@ static ssize_t ctxid_mask_show(struct device *dev,
 	struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct etm_config *config = &drvdata->config;
 
+	/*
+	 * Don't use contextID tracing if coming from a PID namespace.  See
+	 * comment in ctxid_pid_store().
+	 */
+	if (task_active_pid_ns(current) != &init_pid_ns)
+		return -EINVAL;
+
 	val = config->ctxid_mask;
 	return sprintf(buf, "%#lx\n", val);
 }
@@ -1076,6 +1100,13 @@ static ssize_t ctxid_mask_store(struct device *dev,
 	struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct etm_config *config = &drvdata->config;
 
+	/*
+	 * Don't use contextID tracing if coming from a PID namespace.  See
+	 * comment in ctxid_pid_store().
+	 */
+	if (task_active_pid_ns(current) != &init_pid_ns)
+		return -EINVAL;
+
 	ret = kstrtoul(buf, 16, &val);
 	if (ret)
 		return ret;
diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
index 15ed64d51a5b..7c74263c333d 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x.c
@@ -230,10 +230,8 @@ void etm_set_default(struct etm_config *config)
 
 	config->seq_curr_state = 0x0;
 	config->ctxid_idx = 0x0;
-	for (i = 0; i < ETM_MAX_CTXID_CMP; i++) {
+	for (i = 0; i < ETM_MAX_CTXID_CMP; i++)
 		config->ctxid_pid[i] = 0x0;
-		config->ctxid_vpid[i] = 0x0;
-	}
 
 	config->ctxid_mask = 0x0;
 	/* Setting default to 1024 as per TRM recommendation */
-- 
2.7.4

[PATCH 2/3] coresight: etm4x: Don't use contextID with PID namespaces

[Mathieu Poirier]

As with ETM3x, the ETM4x tracers can trigger trace acquisition based on
contextID value, something that isn't useful when PID namespaces are
enabled.  Indeed the PID value of a process has a different representation
in the kernel and the PID namespace, making the feature confusing and
potentially leaking internal kernel information.

As such simply return an error when the feature is being used from a
PID namespace other than the default one.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 .../hwtracing/coresight/coresight-etm4x-sysfs.c    | 47 +++++++++++++++++-----
 drivers/hwtracing/coresight/coresight-etm4x.h      |  3 --
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 4eb8da785ce0..a0365e23678e 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -4,6 +4,7 @@
  * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
  */
 
+#include <linux/pid_namespace.h>
 #include <linux/pm_runtime.h>
 #include <linux/sysfs.h>
 #include "coresight-etm4x.h"
@@ -250,10 +251,8 @@ static ssize_t reset_store(struct device *dev,
 	}
 
 	config->ctxid_idx = 0x0;
-	for (i = 0; i < drvdata->numcidc; i++) {
+	for (i = 0; i < drvdata->numcidc; i++)
 		config->ctxid_pid[i] = 0x0;
-		config->ctxid_vpid[i] = 0x0;
-	}
 
 	config->ctxid_mask0 = 0x0;
 	config->ctxid_mask1 = 0x0;
@@ -1637,9 +1636,16 @@ static ssize_t ctxid_pid_show(struct device *dev,
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct etmv4_config *config = &drvdata->config;
 
+	/*
+	 * Don't use contextID tracing if coming from a PID namespace.  See
+	 * comment in ctxid_pid_store().
+	 */
+	if (task_active_pid_ns(current) != &init_pid_ns)
+		return -EINVAL;
+
 	spin_lock(&drvdata->spinlock);
 	idx = config->ctxid_idx;
-	val = (unsigned long)config->ctxid_vpid[idx];
+	val = (unsigned long)config->ctxid_pid[idx];
 	spin_unlock(&drvdata->spinlock);
 	return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
 }
@@ -1649,26 +1655,35 @@ static ssize_t ctxid_pid_store(struct device *dev,
 			       const char *buf, size_t size)
 {
 	u8 idx;
-	unsigned long vpid, pid;
+	unsigned long pid;
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct etmv4_config *config = &drvdata->config;
 
 	/*
+	 * When contextID tracing is enabled the tracers will insert the
+	 * value found in the contextID register in the trace stream.  But if
+	 * a process is in a namespace the PID of that process as seen from the
+	 * namespace won't be what the kernel sees, something that makes the
+	 * feature confusing and can potentially leak kernel only information.
+	 * As such refuse to use the feature if @current is not in the initial
+	 * PID namespace.
+	 */
+	if (task_active_pid_ns(current) != &init_pid_ns)
+		return -EINVAL;
+
+	/*
 	 * only implemented when ctxid tracing is enabled, i.e. at least one
 	 * ctxid comparator is implemented and ctxid is greater than 0 bits
 	 * in length
 	 */
 	if (!drvdata->ctxid_size || !drvdata->numcidc)
 		return -EINVAL;
-	if (kstrtoul(buf, 16, &vpid))
+	if (kstrtoul(buf, 16, &pid))
 		return -EINVAL;
 
-	pid = coresight_vpid_to_pid(vpid);
-
 	spin_lock(&drvdata->spinlock);
 	idx = config->ctxid_idx;
 	config->ctxid_pid[idx] = (u64)pid;
-	config->ctxid_vpid[idx] = (u64)vpid;
 	spin_unlock(&drvdata->spinlock);
 	return size;
 }
@@ -1682,6 +1697,13 @@ static ssize_t ctxid_masks_show(struct device *dev,
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
 	struct etmv4_config *config = &drvdata->config;
 
+	/*
+	 * Don't use contextID tracing if coming from a PID namespace.  See
+	 * comment in ctxid_pid_store().
+	 */
+	if (task_active_pid_ns(current) != &init_pid_ns)
+		return -EINVAL;
+
 	spin_lock(&drvdata->spinlock);
 	val1 = config->ctxid_mask0;
 	val2 = config->ctxid_mask1;
@@ -1699,6 +1721,13 @@ static ssize_t ctxid_masks_store(struct device *dev,
 	struct etmv4_config *config = &drvdata->config;
 
 	/*
+	 * Don't use contextID tracing if coming from a PID namespace.  See
+	 * comment in ctxid_pid_store().
+	 */
+	if (task_active_pid_ns(current) != &init_pid_ns)
+		return -EINVAL;
+
+	/*
 	 * only implemented when ctxid tracing is enabled, i.e. at least one
 	 * ctxid comparator is implemented and ctxid is greater than 0 bits
 	 * in length
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index b7c4a6f6c6b9..52786e9d8926 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -230,8 +230,6 @@
  * @addr_type:	Current status of the comparator register.
  * @ctxid_idx:	Context ID index selector.
  * @ctxid_pid:	Value of the context ID comparator.
- * @ctxid_vpid:	Virtual PID seen by users if PID namespace is enabled, otherwise
- *		the same value of ctxid_pid.
  * @ctxid_mask0:Context ID comparator mask for comparator 0-3.
  * @ctxid_mask1:Context ID comparator mask for comparator 4-7.
  * @vmid_idx:	VM ID index selector.
@@ -274,7 +272,6 @@ struct etmv4_config {
 	u8				addr_type[ETM_MAX_SINGLE_ADDR_CMP];
 	u8				ctxid_idx;
 	u64				ctxid_pid[ETMv4_MAX_CTXID_CMP];
-	u64				ctxid_vpid[ETMv4_MAX_CTXID_CMP];
 	u32				ctxid_mask0;
 	u32				ctxid_mask1;
 	u8				vmid_idx;
-- 
2.7.4

[PATCH 3/3] coresight: Remove function coresight_vpid_to_pid()

[Mathieu Poirier]

Now that we prevent users from using contextID tracing when PID namespaces
are involved there is no client for function coresight_vpid_to_pid().  As
such simply remove it.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 include/linux/coresight.h | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index c265e0468414..e5421b83e4e6 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -267,24 +267,4 @@ static inline struct coresight_platform_data *of_get_coresight_platform_data(
 	struct device *dev, const struct device_node *node) { return NULL; }
 #endif
 
-#ifdef CONFIG_PID_NS
-static inline unsigned long
-coresight_vpid_to_pid(unsigned long vpid)
-{
-	struct task_struct *task = NULL;
-	unsigned long pid = 0;
-
-	rcu_read_lock();
-	task = find_task_by_vpid(vpid);
-	if (task)
-		pid = task_pid_nr(task);
-	rcu_read_unlock();
-
-	return pid;
-}
-#else
-static inline unsigned long
-coresight_vpid_to_pid(unsigned long vpid) { return vpid; }
-#endif
-
 #endif
-- 
2.7.4

[PATCH 0/3] coresight: Don't use contextID with PID namespaces

[Kim Phillips]

On Tue, 15 May 2018 11:13:34 -0600
Mathieu Poirier <mathieu.poirier@linaro.org> wrote:

> Since the in-kernel value of a PID differs from what is seen from 
> PID namespaces, using contextID tracing with PID namespaces makes the
> feature confusing to use and potentially subject to leaking out internal
> kernel information.
> 
> This set returns an error if contextID and PID namespaces are used in
> conjunction and gets rid of the vpid-to-pid translation function as it
> is no longer needed.

For the series:

Reviewed-by: Kim Phillips <kim.phillips@arm.com>

Thanks,

Kim

[PATCH 0/3] coresight: Don't use contextID with PID namespaces

[Eric W. Biederman]

Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> Since the in-kernel value of a PID differs from what is seen from 
> PID namespaces, using contextID tracing with PID namespaces makes the
> feature confusing to use and potentially subject to leaking out internal
> kernel information.
>
> This set returns an error if contextID and PID namespaces are used in
> conjunction and gets rid of the vpid-to-pid translation function as it
> is no longer needed.

I looked the file can only be written by root.  So limiting things to
the initial pid namespace seems like the right solutions.  Especially as
the trace stream will include the global pid and be available to the
tracer.

This sounds like a simple code with a minimal chance of breaking
userspace.

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>