All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 1/2] perf/pmu: Allow PMU providers to override system-wide security settings
@ 2017-11-17 13:57 ` Tvrtko Ursulin
  0 siblings, 0 replies; 4+ messages in thread
From: Tvrtko Ursulin @ 2017-11-17 13:57 UTC (permalink / raw)
  To: Intel-gfx
  Cc: Peter Zijlstra, linux-kernel, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Ingo Molnar, Namhyung Kim, Jiri Olsa

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

To allow system administrators finer-grained control over security
settings, we add an optional pmu->is_privileged(pmu, event) callback
which is consulted when unprivileged system-wide uncore event collection
is disabled.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 include/linux/perf_event.h |  6 ++++++
 kernel/events/core.c       | 12 +++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 8e22f24ded6a..a93630cad6b9 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -446,6 +446,12 @@ struct pmu {
 	 * Filter events for PMU-specific reasons.
 	 */
 	int (*filter_match)		(struct perf_event *event); /* optional */
+
+	/*
+	 * Returns true if the event needs CAP_SYS_ADMIN privilege in system-
+	 * wide mode.
+	 */
+	bool (*is_privileged)		(struct perf_event *event); /* optional */
 };
 
 /**
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1811dd5aa2e2..adec28c879e8 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3847,6 +3847,16 @@ find_lively_task_by_vpid(pid_t vpid)
 	return task;
 }
 
+static bool cpu_is_privileged(struct pmu *pmu, struct perf_event *event)
+{
+	bool privileged = perf_paranoid_cpu();
+
+	if (privileged && pmu->is_privileged)
+		privileged = pmu->is_privileged(event);
+
+	return privileged;
+}
+
 /*
  * Returns a matching context with refcount and pincount.
  */
@@ -3863,7 +3873,7 @@ find_get_context(struct pmu *pmu, struct task_struct *task,
 
 	if (!task) {
 		/* Must be root to operate on a CPU event: */
-		if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
+		if (cpu_is_privileged(pmu, event) && !capable(CAP_SYS_ADMIN))
 			return ERR_PTR(-EACCES);
 
 		cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [RFC 1/2] perf/pmu: Allow PMU providers to override system-wide security settings
@ 2017-11-17 13:57 ` Tvrtko Ursulin
  0 siblings, 0 replies; 4+ messages in thread
From: Tvrtko Ursulin @ 2017-11-17 13:57 UTC (permalink / raw)
  To: Intel-gfx
  Cc: linux-kernel, Tvrtko Ursulin, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Dmitry Rogozhkin, Chris Wilson

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

To allow system administrators finer-grained control over security
settings, we add an optional pmu->is_privileged(pmu, event) callback
which is consulted when unprivileged system-wide uncore event collection
is disabled.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 include/linux/perf_event.h |  6 ++++++
 kernel/events/core.c       | 12 +++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 8e22f24ded6a..a93630cad6b9 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -446,6 +446,12 @@ struct pmu {
 	 * Filter events for PMU-specific reasons.
 	 */
 	int (*filter_match)		(struct perf_event *event); /* optional */
+
+	/*
+	 * Returns true if the event needs CAP_SYS_ADMIN privilege in system-
+	 * wide mode.
+	 */
+	bool (*is_privileged)		(struct perf_event *event); /* optional */
 };
 
 /**
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1811dd5aa2e2..adec28c879e8 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3847,6 +3847,16 @@ find_lively_task_by_vpid(pid_t vpid)
 	return task;
 }
 
+static bool cpu_is_privileged(struct pmu *pmu, struct perf_event *event)
+{
+	bool privileged = perf_paranoid_cpu();
+
+	if (privileged && pmu->is_privileged)
+		privileged = pmu->is_privileged(event);
+
+	return privileged;
+}
+
 /*
  * Returns a matching context with refcount and pincount.
  */
@@ -3863,7 +3873,7 @@ find_get_context(struct pmu *pmu, struct task_struct *task,
 
 	if (!task) {
 		/* Must be root to operate on a CPU event: */
-		if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
+		if (cpu_is_privileged(pmu, event) && !capable(CAP_SYS_ADMIN))
 			return ERR_PTR(-EACCES);
 
 		cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [RFC 2/2] drm/i915/pmu: Allow fine-grained PMU access control
  2017-11-17 13:57 ` Tvrtko Ursulin
@ 2017-11-17 13:57   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 4+ messages in thread
From: Tvrtko Ursulin @ 2017-11-17 13:57 UTC (permalink / raw)
  To: Intel-gfx; +Cc: linux-kernel

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We implement the new pmu->is_privileged callback and add our own sysctl
as /proc/sys/dev/i915/pmu_stream_paranoid (defaulting to true), which
enables system administrators to override the global
/proc/sys/kernel/perf_event_paranoid setting for i915 PMU only.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_pmu.c | 55 +++++++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_pmu.h |  4 +++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index baa663641bb7..a8eca7303b7e 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -43,6 +43,11 @@
 
 static cpumask_t i915_pmu_cpumask = CPU_MASK_NONE;
 
+/* for sysctl proc_dointvec_minmax of dev.i915.pmu_stream_paranoid */
+static int zero;
+static int one = 1;
+static u32 i915_pmu_stream_paranoid = true;
+
 static u8 engine_config_sample(u64 config)
 {
 	return config & I915_PMU_SAMPLE_MASK;
@@ -647,6 +652,11 @@ static int i915_pmu_event_event_idx(struct perf_event *event)
 	return 0;
 }
 
+static bool i915_pmu_is_privileged(struct perf_event *event)
+{
+	return i915_pmu_stream_paranoid;
+}
+
 static ssize_t i915_pmu_format_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
@@ -834,11 +844,44 @@ static void i915_pmu_unregister_cpuhp_state(struct drm_i915_private *i915)
 #endif
 }
 
+static struct ctl_table pmu_table[] = {
+	{
+		.procname = "pmu_stream_paranoid",
+		.data = &i915_pmu_stream_paranoid,
+		.maxlen = sizeof(i915_pmu_stream_paranoid),
+		.mode = 0644,
+		.proc_handler = proc_dointvec_minmax,
+		.extra1 = &zero,
+		.extra2 = &one,
+	},
+	{}
+};
+
+static struct ctl_table i915_root[] = {
+	{
+		.procname = "i915",
+		.maxlen = 0,
+		.mode = 0555,
+		.child = pmu_table,
+	},
+	{}
+};
+
+static struct ctl_table dev_root[] = {
+	{
+		.procname = "dev",
+		.maxlen = 0,
+		.mode = 0555,
+		.child = i915_root,
+	},
+	{}
+};
+
 void i915_pmu_register(struct drm_i915_private *i915)
 {
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
-	int ret;
+	int ret = -EINVAL;
 
 	if (INTEL_GEN(i915) <= 2) {
 		DRM_INFO("PMU not supported for this GPU.");
@@ -854,6 +897,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
 	i915->pmu.base.stop		= i915_pmu_event_stop;
 	i915->pmu.base.read		= i915_pmu_event_read;
 	i915->pmu.base.event_idx	= i915_pmu_event_event_idx;
+	i915->pmu.base.is_privileged	= i915_pmu_is_privileged;
 
 	spin_lock_init(&i915->pmu.lock);
 	hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
@@ -863,9 +907,12 @@ void i915_pmu_register(struct drm_i915_private *i915)
 		INIT_DELAYED_WORK(&engine->pmu.disable_busy_stats,
 				  __disable_busy_stats);
 
+	if (!(i915->pmu.sysctl_header = register_sysctl_table(dev_root)))
+		goto err;
+
 	ret = perf_pmu_register(&i915->pmu.base, "i915", -1);
 	if (ret)
-		goto err;
+		goto err_pmu;
 
 	ret = i915_pmu_register_cpuhp_state(i915);
 	if (ret)
@@ -875,6 +922,8 @@ void i915_pmu_register(struct drm_i915_private *i915)
 
 err_unreg:
 	perf_pmu_unregister(&i915->pmu.base);
+err_pmu:
+	unregister_sysctl_table(i915->pmu.sysctl_header);
 err:
 	i915->pmu.base.event_init = NULL;
 	DRM_NOTE("Failed to register PMU! (err=%d)\n", ret);
@@ -899,6 +948,8 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
 
 	i915_pmu_unregister_cpuhp_state(i915);
 
+	unregister_sysctl_table(i915->pmu.sysctl_header);
+
 	perf_pmu_unregister(&i915->pmu.base);
 	i915->pmu.base.event_init = NULL;
 }
diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
index e209783a4c5f..30e2192b4218 100644
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -61,6 +61,10 @@ struct i915_pmu {
 	 * @timer: Timer for internal i915 PMU sampling.
 	 */
 	struct hrtimer timer;
+	/**
+	 * @sysctl_header: Sysctl table header.
+	 */
+	struct ctl_table_header *sysctl_header;
 	/**
 	 * @enable: Bitmask of all currently enabled events.
 	 *
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [RFC 2/2] drm/i915/pmu: Allow fine-grained PMU access control
@ 2017-11-17 13:57   ` Tvrtko Ursulin
  0 siblings, 0 replies; 4+ messages in thread
From: Tvrtko Ursulin @ 2017-11-17 13:57 UTC (permalink / raw)
  To: Intel-gfx; +Cc: linux-kernel, Tvrtko Ursulin, Dmitry Rogozhkin, Chris Wilson

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We implement the new pmu->is_privileged callback and add our own sysctl
as /proc/sys/dev/i915/pmu_stream_paranoid (defaulting to true), which
enables system administrators to override the global
/proc/sys/kernel/perf_event_paranoid setting for i915 PMU only.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_pmu.c | 55 +++++++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_pmu.h |  4 +++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index baa663641bb7..a8eca7303b7e 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -43,6 +43,11 @@
 
 static cpumask_t i915_pmu_cpumask = CPU_MASK_NONE;
 
+/* for sysctl proc_dointvec_minmax of dev.i915.pmu_stream_paranoid */
+static int zero;
+static int one = 1;
+static u32 i915_pmu_stream_paranoid = true;
+
 static u8 engine_config_sample(u64 config)
 {
 	return config & I915_PMU_SAMPLE_MASK;
@@ -647,6 +652,11 @@ static int i915_pmu_event_event_idx(struct perf_event *event)
 	return 0;
 }
 
+static bool i915_pmu_is_privileged(struct perf_event *event)
+{
+	return i915_pmu_stream_paranoid;
+}
+
 static ssize_t i915_pmu_format_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
@@ -834,11 +844,44 @@ static void i915_pmu_unregister_cpuhp_state(struct drm_i915_private *i915)
 #endif
 }
 
+static struct ctl_table pmu_table[] = {
+	{
+		.procname = "pmu_stream_paranoid",
+		.data = &i915_pmu_stream_paranoid,
+		.maxlen = sizeof(i915_pmu_stream_paranoid),
+		.mode = 0644,
+		.proc_handler = proc_dointvec_minmax,
+		.extra1 = &zero,
+		.extra2 = &one,
+	},
+	{}
+};
+
+static struct ctl_table i915_root[] = {
+	{
+		.procname = "i915",
+		.maxlen = 0,
+		.mode = 0555,
+		.child = pmu_table,
+	},
+	{}
+};
+
+static struct ctl_table dev_root[] = {
+	{
+		.procname = "dev",
+		.maxlen = 0,
+		.mode = 0555,
+		.child = i915_root,
+	},
+	{}
+};
+
 void i915_pmu_register(struct drm_i915_private *i915)
 {
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
-	int ret;
+	int ret = -EINVAL;
 
 	if (INTEL_GEN(i915) <= 2) {
 		DRM_INFO("PMU not supported for this GPU.");
@@ -854,6 +897,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
 	i915->pmu.base.stop		= i915_pmu_event_stop;
 	i915->pmu.base.read		= i915_pmu_event_read;
 	i915->pmu.base.event_idx	= i915_pmu_event_event_idx;
+	i915->pmu.base.is_privileged	= i915_pmu_is_privileged;
 
 	spin_lock_init(&i915->pmu.lock);
 	hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
@@ -863,9 +907,12 @@ void i915_pmu_register(struct drm_i915_private *i915)
 		INIT_DELAYED_WORK(&engine->pmu.disable_busy_stats,
 				  __disable_busy_stats);
 
+	if (!(i915->pmu.sysctl_header = register_sysctl_table(dev_root)))
+		goto err;
+
 	ret = perf_pmu_register(&i915->pmu.base, "i915", -1);
 	if (ret)
-		goto err;
+		goto err_pmu;
 
 	ret = i915_pmu_register_cpuhp_state(i915);
 	if (ret)
@@ -875,6 +922,8 @@ void i915_pmu_register(struct drm_i915_private *i915)
 
 err_unreg:
 	perf_pmu_unregister(&i915->pmu.base);
+err_pmu:
+	unregister_sysctl_table(i915->pmu.sysctl_header);
 err:
 	i915->pmu.base.event_init = NULL;
 	DRM_NOTE("Failed to register PMU! (err=%d)\n", ret);
@@ -899,6 +948,8 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
 
 	i915_pmu_unregister_cpuhp_state(i915);
 
+	unregister_sysctl_table(i915->pmu.sysctl_header);
+
 	perf_pmu_unregister(&i915->pmu.base);
 	i915->pmu.base.event_init = NULL;
 }
diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
index e209783a4c5f..30e2192b4218 100644
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -61,6 +61,10 @@ struct i915_pmu {
 	 * @timer: Timer for internal i915 PMU sampling.
 	 */
 	struct hrtimer timer;
+	/**
+	 * @sysctl_header: Sysctl table header.
+	 */
+	struct ctl_table_header *sysctl_header;
 	/**
 	 * @enable: Bitmask of all currently enabled events.
 	 *
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-11-17 13:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-17 13:57 [RFC 1/2] perf/pmu: Allow PMU providers to override system-wide security settings Tvrtko Ursulin
2017-11-17 13:57 ` Tvrtko Ursulin
2017-11-17 13:57 ` [RFC 2/2] drm/i915/pmu: Allow fine-grained PMU access control Tvrtko Ursulin
2017-11-17 13:57   ` Tvrtko Ursulin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.