[PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

[PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

[Sai Prakash Ranjan]

Hardware assisted tracing families such as ARM Coresight, Intel PT
provides rich tracing capabilities including instruction level
tracing and accurate timestamps which are very useful for profiling
and also pose a significant security risk. One such example of
security risk is when kernel mode tracing is not excluded and these
hardware assisted tracing can be used to analyze cryptographic code
execution. In this case, even the root user must not be able to infer
anything.

To explain it more clearly in the words of a security team member
(credits: Mattias Nissler),

"Consider a system where disk contents are encrypted and the encryption
key is set up by the user when mounting the file system. From that point
on the encryption key resides in the kernel. It seems reasonable to
expect that the disk encryption key be protected from exfiltration even
if the system later suffers a root compromise (or even against insiders
that have root access), at least as long as the attacker doesn't
manage to compromise the kernel."

Here the idea is to protect such important information from all users
including root users since root privileges does not have to mean full
control over the kernel [1] and root compromise does not have to be
the end of the world.

But "Peter said even the regular counters can be used for full branch trace,
the information isn't as accurate as PT and friends and not easier but
is good enough to infer plenty". This would mean that a global tunable
config for all kernel mode pmu tracing is more appropriate than the one
targeting the hardware assisted instruction tracing.

Currently we can exclude kernel mode tracing via perf_event_paranoid
sysctl but it has following limitations,

 * No option to restrict kernel mode instruction tracing by the
   root user.
 * Not possible to restrict kernel mode instruction tracing when the
   hardware assisted tracing IPs like ARM Coresight ETMs use an
   additional interface via sysfs for tracing in addition to perf
   interface.

So introduce a new config CONFIG_EXCLUDE_KERNEL_PMU_TRACE to exclude
kernel mode pmu tracing for all users including root which will be
generic and applicable to all hardware tracing families and which
can also be used with other interfaces like sysfs in case of ETMs.

Patch 1 adds this new config and the support in perf core to exclude
all kernel mode PMU tracing.

Patch 2 adds the perf evsel warning message when the perf tool users
attempt to perform a kernel mode trace with the config enabled to
exclude the kernel mode tracing.

Patch 3 and Patch 4 adds the support for excluding kernel mode for
ARM Coresight ETM{4,3}XX sysfs mode using the newly introduced generic
config.

[1] https://lwn.net/Articles/796866/

Changes in v2:
 * Move from kernel mode instruction tracing to all kernel level PMU tracing (Peter)
 * Move the check and warning to the caller mode_store() (Doug)

Sai Prakash Ranjan (4):
  perf/core: Add support to exclude kernel mode PMU tracing
  perf evsel: Print warning for excluding kernel mode instruction
    tracing
  coresight: etm4x: Add support to exclude kernel mode tracing
  coresight: etm3x: Add support to exclude kernel mode tracing

 drivers/hwtracing/coresight/coresight-etm3x-core.c  |  3 +++
 drivers/hwtracing/coresight/coresight-etm3x-sysfs.c |  6 ++++++
 drivers/hwtracing/coresight/coresight-etm4x-core.c  |  6 +++++-
 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c |  6 ++++++
 init/Kconfig                                        | 11 +++++++++++
 kernel/events/core.c                                |  3 +++
 tools/perf/util/evsel.c                             |  3 ++-
 7 files changed, 36 insertions(+), 2 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 1/4] perf/core: Add support to exclude kernel mode PMU tracing

[Sai Prakash Ranjan]

Hardware assisted tracing families such as ARM Coresight, Intel PT
provides rich tracing capabilities including instruction level
tracing and accurate timestamps which are very useful for profiling
and also pose a significant security risk. One such example of
security risk is when kernel mode tracing is not excluded and these
hardware assisted tracing can be used to analyze cryptographic code
execution. In this case, even the root user must not be able to infer
anything.

To explain it more clearly in the words of a security team member
(credits: Mattias Nissler),

"Consider a system where disk contents are encrypted and the encryption
key is set up by the user when mounting the file system. From that point
on the encryption key resides in the kernel. It seems reasonable to
expect that the disk encryption key be protected from exfiltration even
if the system later suffers a root compromise (or even against insiders
that have root access), at least as long as the attacker doesn't
manage to compromise the kernel."

Here the idea is to protect such important information from all users
including root users since root privileges does not have to mean full
control over the kernel [1] and root compromise does not have to be
the end of the world.

But "Peter said even the regular counters can be used for full branch
trace, the information isn't as accurate as PT and friends and not easier
but is good enough to infer plenty". This would mean that a global tunable
config for all kernel mode pmu tracing is more appropriate than the one
targeting the hardware assisted instruction tracing.

Currently we can exclude kernel mode tracing via perf_event_paranoid
sysctl but it has following limitations,

 * No option to restrict kernel mode instruction tracing by the
   root user.
 * Not possible to restrict kernel mode instruction tracing when the
   hardware assisted tracing IPs like ARM Coresight ETMs use an
   additional interface via sysfs for tracing in addition to perf
   interface.

So introduce a new config CONFIG_EXCLUDE_KERNEL_PMU_TRACE to exclude
kernel mode pmu tracing which will be generic and applicable to all
hardware tracing families and which can also be used with other
interfaces like sysfs in case of ETMs.

[1] https://lwn.net/Articles/796866/

Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Suggested-by: Al Grant <al.grant@arm.com>
Tested-by: Denis Nikitin <denik@chromium.org>
Link: https://lore.kernel.org/lkml/20201015124522.1876-1-saiprakash.ranjan@codeaurora.org/
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 init/Kconfig         | 11 +++++++++++
 kernel/events/core.c |  3 +++
 2 files changed, 14 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index 22946fe5ded9..34d9b7587d2e 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1848,6 +1848,17 @@ config DEBUG_PERF_USE_VMALLOC
 
 endmenu
 
+config EXCLUDE_KERNEL_PMU_TRACE
+	bool "Exclude Kernel mode PMU tracing"
+	depends on PERF_EVENTS
+	help
+	  Exclude Kernel mode PMU tracing for all users.
+
+	  This option allows to disable kernel mode tracing for all
+	  users(including root) which is especially useful in production
+	  systems where only userspace tracing might be preferred for
+	  security reasons.
+
 config VM_EVENT_COUNTERS
 	default y
 	bool "Enable VM event counters for /proc/vmstat" if EXPERT
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 0aeca5f3c0ac..241cc9640483 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11770,6 +11770,9 @@ SYSCALL_DEFINE5(perf_event_open,
 	if (err)
 		return err;
 
+	if (IS_ENABLED(CONFIG_EXCLUDE_KERNEL_PMU_TRACE) && !attr.exclude_kernel)
+		return -EACCES;
+
 	if (!attr.exclude_kernel) {
 		err = perf_allow_kernel(&attr);
 		if (err)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 2/4] perf evsel: Print warning for excluding kernel mode instruction tracing

[Sai Prakash Ranjan]

Add a warning message to check CONFIG_EXCLUDE_KERNEL_HW_ITRACE kernel
config which excludes kernel mode instruction tracing to help perf tool
users identify the perf event open failure when they attempt kernel mode
tracing with this config enabled.

Tested-by: Denis Nikitin <denik@chromium.org>
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 tools/perf/util/evsel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 1bf76864c4f2..3f584128a590 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2672,7 +2672,8 @@ int evsel__open_strerror(struct evsel *evsel, struct target *target,
 		 ">= 1: Disallow CPU event access\n"
 		 ">= 2: Disallow kernel profiling\n"
 		 "To make the adjusted perf_event_paranoid setting permanent preserve it\n"
-		 "in /etc/sysctl.conf (e.g. kernel.perf_event_paranoid = <setting>)",
+		 "in /etc/sysctl.conf (e.g. kernel.perf_event_paranoid = <setting>)\n\n"
+		 "Also check CONFIG_EXCLUDE_KERNEL_PMU_TRACE if kernel mode tracing is allowed.",
 		 perf_event_paranoid());
 	case ENOENT:
 		return scnprintf(msg, size, "The %s event is not supported.", evsel__name(evsel));
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 3/4] coresight: etm4x: Add support to exclude kernel mode tracing

[Sai Prakash Ranjan]

On production systems with ETMs enabled, it is preferred to exclude
kernel mode(NS EL1) tracing for security concerns and support only
userspace(NS EL0) tracing. Perf subsystem interface uses the newly
introduced kernel config CONFIG_EXCLUDE_KERNEL_PMU_TRACE to exclude
kernel mode tracing, but there is an additional interface via sysfs
for ETMs which also needs to be handled to exclude kernel
mode tracing. So we use this same generic kernel config to handle
the sysfs mode of tracing. This config is disabled by default and
would not affect the current configuration which has both kernel and
userspace tracing enabled by default.

Tested-by: Denis Nikitin <denik@chromium.org>
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c  | 6 +++++-
 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 6 ++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 15016f757828..dbd6b96f3a78 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1235,12 +1235,16 @@ static void etm4_set_default(struct etmv4_config *config)
 		return;
 
 	/*
-	 * Make default initialisation trace everything
+	 * Make default initialisation trace everything when
+	 * CONFIG_EXCLUDE_KERNEL_PMU_TRACE is disabled.
 	 *
 	 * This is done by a minimum default config sufficient to enable
 	 * full instruction trace - with a default filter for trace all
 	 * achieved by having no filtering.
 	 */
+	if (IS_ENABLED(CONFIG_EXCLUDE_KERNEL_PMU_TRACE))
+		config->mode |= ETM_MODE_EXCL_KERN;
+
 	etm4_set_default_config(config);
 	etm4_set_default_filter(config);
 }
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 0995a10790f4..78ba2a0888fc 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -296,6 +296,12 @@ static ssize_t mode_store(struct device *dev,
 	if (kstrtoul(buf, 16, &val))
 		return -EINVAL;
 
+	if (IS_ENABLED(CONFIG_EXCLUDE_KERNEL_PMU_TRACE) && (!(val & ETM_MODE_EXCL_KERN))) {
+		dev_warn(dev,
+			"Kernel mode tracing is not allowed, check your kernel config\n");
+		return -EACCES;
+	}
+
 	spin_lock(&drvdata->spinlock);
 	config->mode = val & ETMv4_MODE_ALL;
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCHv2 4/4] coresight: etm3x: Add support to exclude kernel mode tracing

[Sai Prakash Ranjan]

On production systems with ETMs enabled, it is preferred to exclude
kernel mode(NS EL1) tracing for security concerns and support only
userspace(NS EL0) tracing. Perf subsystem interface uses the newly
introduced kernel config CONFIG_EXCLUDE_KERNEL_PMU_TRACE to exclude
kernel mode tracing, but there is an additional interface
via sysfs for ETMs which also needs to be handled to exclude kernel
mode tracing. So we use this same generic kernel config to handle
the sysfs mode of tracing. This config is disabled by default and
would not affect the current configuration which has both kernel and
userspace tracing enabled by default.

Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 drivers/hwtracing/coresight/coresight-etm3x-core.c  | 3 +++
 drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index cf64ce73a741..d94f6b01ca09 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -195,6 +195,9 @@ void etm_set_default(struct etm_config *config)
 	if (WARN_ON_ONCE(!config))
 		return;
 
+	if (IS_ENABLED(CONFIG_EXCLUDE_KERNEL_PMU_TRACE))
+		config->mode |= ETM_MODE_EXCL_KERN;
+
 	/*
 	 * Taken verbatim from the TRM:
 	 *
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index e8c7649f123e..f522fc2e01b3 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -116,6 +116,12 @@ static ssize_t mode_store(struct device *dev,
 	if (ret)
 		return ret;
 
+	if (IS_ENABLED(CONFIG_EXCLUDE_KERNEL_PMU_TRACE) && (!(val & ETM_MODE_EXCL_KERN))) {
+		dev_warn(dev,
+			"Kernel mode tracing is not allowed, check your kernel config\n");
+		return -EACCES;
+	}
+
 	spin_lock(&drvdata->spinlock);
 	config->mode = val & ETM_MODE_ALL;
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 1/4] perf/core: Add support to exclude kernel mode PMU tracing

[Doug Anderson]

Hi,

On Mon, Mar 1, 2021 at 11:05 AM Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> Hardware assisted tracing families such as ARM Coresight, Intel PT
> provides rich tracing capabilities including instruction level
> tracing and accurate timestamps which are very useful for profiling
> and also pose a significant security risk. One such example of
> security risk is when kernel mode tracing is not excluded and these
> hardware assisted tracing can be used to analyze cryptographic code
> execution. In this case, even the root user must not be able to infer
> anything.
>
> To explain it more clearly in the words of a security team member
> (credits: Mattias Nissler),
>
> "Consider a system where disk contents are encrypted and the encryption
> key is set up by the user when mounting the file system. From that point
> on the encryption key resides in the kernel. It seems reasonable to
> expect that the disk encryption key be protected from exfiltration even
> if the system later suffers a root compromise (or even against insiders
> that have root access), at least as long as the attacker doesn't
> manage to compromise the kernel."
>
> Here the idea is to protect such important information from all users
> including root users since root privileges does not have to mean full
> control over the kernel [1] and root compromise does not have to be
> the end of the world.
>
> But "Peter said even the regular counters can be used for full branch
> trace, the information isn't as accurate as PT and friends and not easier
> but is good enough to infer plenty". This would mean that a global tunable
> config for all kernel mode pmu tracing is more appropriate than the one
> targeting the hardware assisted instruction tracing.
>
> Currently we can exclude kernel mode tracing via perf_event_paranoid
> sysctl but it has following limitations,
>
>  * No option to restrict kernel mode instruction tracing by the
>    root user.
>  * Not possible to restrict kernel mode instruction tracing when the
>    hardware assisted tracing IPs like ARM Coresight ETMs use an
>    additional interface via sysfs for tracing in addition to perf
>    interface.
>
> So introduce a new config CONFIG_EXCLUDE_KERNEL_PMU_TRACE to exclude
> kernel mode pmu tracing which will be generic and applicable to all
> hardware tracing families and which can also be used with other
> interfaces like sysfs in case of ETMs.
>
> [1] https://lwn.net/Articles/796866/
>
> Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Suggested-by: Al Grant <al.grant@arm.com>
> Tested-by: Denis Nikitin <denik@chromium.org>
> Link: https://lore.kernel.org/lkml/20201015124522.1876-1-saiprakash.ranjan@codeaurora.org/
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
>  init/Kconfig         | 11 +++++++++++
>  kernel/events/core.c |  3 +++
>  2 files changed, 14 insertions(+)

I'm not really knowledgeable at all about the perf subsystem so my
review doesn't hold a lot of weight.  However, Sai's patch seems sane
to me.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCHv2 2/4] perf evsel: Print warning for excluding kernel mode instruction tracing

[Doug Anderson]

Hi,

On Mon, Mar 1, 2021 at 11:05 AM Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> Add a warning message to check CONFIG_EXCLUDE_KERNEL_HW_ITRACE kernel
> config which excludes kernel mode instruction tracing to help perf tool
> users identify the perf event open failure when they attempt kernel mode
> tracing with this config enabled.
>
> Tested-by: Denis Nikitin <denik@chromium.org>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
>  tools/perf/util/evsel.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

I'm not really knowledgeable at all about the perf subsystem so my
review doesn't hold a lot of weight.  However, Sai's patch seems sane
to me.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCHv2 3/4] coresight: etm4x: Add support to exclude kernel mode tracing

[Doug Anderson]

Hi,

On Mon, Mar 1, 2021 at 11:05 AM Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> On production systems with ETMs enabled, it is preferred to exclude
> kernel mode(NS EL1) tracing for security concerns and support only
> userspace(NS EL0) tracing. Perf subsystem interface uses the newly
> introduced kernel config CONFIG_EXCLUDE_KERNEL_PMU_TRACE to exclude
> kernel mode tracing, but there is an additional interface via sysfs
> for ETMs which also needs to be handled to exclude kernel
> mode tracing. So we use this same generic kernel config to handle
> the sysfs mode of tracing. This config is disabled by default and
> would not affect the current configuration which has both kernel and
> userspace tracing enabled by default.
>
> Tested-by: Denis Nikitin <denik@chromium.org>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c  | 6 +++++-
>  drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 6 ++++++
>  2 files changed, 11 insertions(+), 1 deletion(-)

Not that I'm an expert in the perf subsystem, but the concern I had
with v1 is now addressed.  FWIW this seems fine to me now.

Reviewed-by: Douglas Anderson <dianders@chromium.org>


> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -296,6 +296,12 @@ static ssize_t mode_store(struct device *dev,
>         if (kstrtoul(buf, 16, &val))
>                 return -EINVAL;
>
> +       if (IS_ENABLED(CONFIG_EXCLUDE_KERNEL_PMU_TRACE) && (!(val & ETM_MODE_EXCL_KERN))) {
> +               dev_warn(dev,
> +                       "Kernel mode tracing is not allowed, check your kernel config\n");

slight nit that I think your string needs to be indented by 1 space.  ;-)

Re: [PATCHv2 4/4] coresight: etm3x: Add support to exclude kernel mode tracing

[Doug Anderson]

Hi,

On Mon, Mar 1, 2021 at 11:05 AM Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> On production systems with ETMs enabled, it is preferred to exclude
> kernel mode(NS EL1) tracing for security concerns and support only
> userspace(NS EL0) tracing. Perf subsystem interface uses the newly
> introduced kernel config CONFIG_EXCLUDE_KERNEL_PMU_TRACE to exclude
> kernel mode tracing, but there is an additional interface
> via sysfs for ETMs which also needs to be handled to exclude kernel
> mode tracing. So we use this same generic kernel config to handle
> the sysfs mode of tracing. This config is disabled by default and
> would not affect the current configuration which has both kernel and
> userspace tracing enabled by default.
>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
>  drivers/hwtracing/coresight/coresight-etm3x-core.c  | 3 +++
>  drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 6 ++++++
>  2 files changed, 9 insertions(+)

Reviewed-by: Douglas Anderson <dianders@chromium.org>


> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> index e8c7649f123e..f522fc2e01b3 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> @@ -116,6 +116,12 @@ static ssize_t mode_store(struct device *dev,
>         if (ret)
>                 return ret;
>
> +       if (IS_ENABLED(CONFIG_EXCLUDE_KERNEL_PMU_TRACE) && (!(val & ETM_MODE_EXCL_KERN))) {
> +               dev_warn(dev,
> +                       "Kernel mode tracing is not allowed, check your kernel config\n");

Same nit as in patch #3 that the above string should be indented by 1
more space.

Re: [PATCHv2 3/4] coresight: etm4x: Add support to exclude kernel mode tracing

[Sai Prakash Ranjan]

On 2021-03-02 04:13, Doug Anderson wrote:
> Hi,
> 
> On Mon, Mar 1, 2021 at 11:05 AM Sai Prakash Ranjan
> <saiprakash.ranjan@codeaurora.org> wrote:
>> 
>> On production systems with ETMs enabled, it is preferred to exclude
>> kernel mode(NS EL1) tracing for security concerns and support only
>> userspace(NS EL0) tracing. Perf subsystem interface uses the newly
>> introduced kernel config CONFIG_EXCLUDE_KERNEL_PMU_TRACE to exclude
>> kernel mode tracing, but there is an additional interface via sysfs
>> for ETMs which also needs to be handled to exclude kernel
>> mode tracing. So we use this same generic kernel config to handle
>> the sysfs mode of tracing. This config is disabled by default and
>> would not affect the current configuration which has both kernel and
>> userspace tracing enabled by default.
>> 
>> Tested-by: Denis Nikitin <denik@chromium.org>
>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> ---
>>  drivers/hwtracing/coresight/coresight-etm4x-core.c  | 6 +++++-
>>  drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 6 ++++++
>>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> Not that I'm an expert in the perf subsystem, but the concern I had
> with v1 is now addressed.  FWIW this seems fine to me now.
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 

Thanks Doug.

> 
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
>> @@ -296,6 +296,12 @@ static ssize_t mode_store(struct device *dev,
>>         if (kstrtoul(buf, 16, &val))
>>                 return -EINVAL;
>> 
>> +       if (IS_ENABLED(CONFIG_EXCLUDE_KERNEL_PMU_TRACE) && (!(val & 
>> ETM_MODE_EXCL_KERN))) {
>> +               dev_warn(dev,
>> +                       "Kernel mode tracing is not allowed, check 
>> your kernel config\n");
> 
> slight nit that I think your string needs to be indented by 1 space.  
> ;-)
> 

Ah yes, I will have to post v3 anyways to fix commit message in Patch 2
after I get few more feedback for other patches, I will fix this up as 
well.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 2/4] perf evsel: Print warning for excluding kernel mode instruction tracing

[Sai Prakash Ranjan]

Hi

On 2021-03-02 04:13, Doug Anderson wrote:
> Hi,
> 
> On Mon, Mar 1, 2021 at 11:05 AM Sai Prakash Ranjan
> <saiprakash.ranjan@codeaurora.org> wrote:
>> 
>> Add a warning message to check CONFIG_EXCLUDE_KERNEL_HW_ITRACE kernel
>> config which excludes kernel mode instruction tracing to help perf 
>> tool
>> users identify the perf event open failure when they attempt kernel 
>> mode
>> tracing with this config enabled.
>> 
>> Tested-by: Denis Nikitin <denik@chromium.org>
>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> ---
>>  tools/perf/util/evsel.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> I'm not really knowledgeable at all about the perf subsystem so my
> review doesn't hold a lot of weight.  However, Sai's patch seems sane
> to me.
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Thanks for the review, I carefully modified all the patch commit 
messages
to change the config and wording from CONFIG_EXCLUDE_KERNEL_HW_ITRACE to
CONFIG_EXCLUDE_KERNEL_PMU_TRACE but missed this commit message. I will
fix this up in v3, but will hold off posting v3 till I get some feedback
on other patches.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 4/4] coresight: etm3x: Add support to exclude kernel mode tracing

[Sai Prakash Ranjan]

On 2021-03-02 04:13, Doug Anderson wrote:
> Hi,
> 
> On Mon, Mar 1, 2021 at 11:05 AM Sai Prakash Ranjan
> <saiprakash.ranjan@codeaurora.org> wrote:
>> 
>> On production systems with ETMs enabled, it is preferred to exclude
>> kernel mode(NS EL1) tracing for security concerns and support only
>> userspace(NS EL0) tracing. Perf subsystem interface uses the newly
>> introduced kernel config CONFIG_EXCLUDE_KERNEL_PMU_TRACE to exclude
>> kernel mode tracing, but there is an additional interface
>> via sysfs for ETMs which also needs to be handled to exclude kernel
>> mode tracing. So we use this same generic kernel config to handle
>> the sysfs mode of tracing. This config is disabled by default and
>> would not affect the current configuration which has both kernel and
>> userspace tracing enabled by default.
>> 
>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> ---
>>  drivers/hwtracing/coresight/coresight-etm3x-core.c  | 3 +++
>>  drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 6 ++++++
>>  2 files changed, 9 insertions(+)
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 

Thanks.

> 
>> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c 
>> b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
>> index e8c7649f123e..f522fc2e01b3 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
>> @@ -116,6 +116,12 @@ static ssize_t mode_store(struct device *dev,
>>         if (ret)
>>                 return ret;
>> 
>> +       if (IS_ENABLED(CONFIG_EXCLUDE_KERNEL_PMU_TRACE) && (!(val & 
>> ETM_MODE_EXCL_KERN))) {
>> +               dev_warn(dev,
>> +                       "Kernel mode tracing is not allowed, check 
>> your kernel config\n");
> 
> Same nit as in patch #3 that the above string should be indented by 1
> more space.
> 

Will fix this up as well in v3.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

[Andi Kleen]

Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> writes:
>
> "Consider a system where disk contents are encrypted and the encryption
> key is set up by the user when mounting the file system. From that point
> on the encryption key resides in the kernel. It seems reasonable to
> expect that the disk encryption key be protected from exfiltration even
> if the system later suffers a root compromise (or even against insiders
> that have root access), at least as long as the attacker doesn't
> manage to compromise the kernel."

Normally disk encryption is in specialized work queues. It's total
overkill to restrict all of the kernel if you just want to restrict
those work queues.

I would suggest some more analysis where secrets are actually stored
and handled first.

-Andi

Re: [PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

[Andi Kleen]

Andi Kleen <ak@linux.intel.com> writes:
>
> Normally disk encryption is in specialized work queues. It's total
> overkill to restrict all of the kernel if you just want to restrict
> those work queues.
>
> I would suggest some more analysis where secrets are actually stored
> and handled first.

Also thinking about this more:

You really only want to limit data tracing here.

If tracing branches could reveal secrets the crypto code would be
already insecure due to timing side channels. If that's the
case it would already require fixing the crypto code.

-Andi

Re: [PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

[Sai Prakash Ranjan]

Hi Andi,

On 2021-03-05 01:47, Andi Kleen wrote:
> Andi Kleen <ak@linux.intel.com> writes:
>>
>> Normally disk encryption is in specialized work queues. It's total
>> overkill to restrict all of the kernel if you just want to restrict
>> those work queues.
>>
>> I would suggest some more analysis where secrets are actually stored
>> and handled first.
> 
> Also thinking about this more:
> 
> You really only want to limit data tracing here.
> 
> If tracing branches could reveal secrets the crypto code would be
> already insecure due to timing side channels. If that's the
> case it would already require fixing the crypto code.
> 

The disk encryption is just one example and there might be others which
we might not be aware of yet and we are not suspecting there is something
wrong with the crypto code that needs to be fixed. Here the idea was to
restrict an external(in the sense that its not related to crypto or any
other security related component) entity such as hardware assisted tracing
like ARM coresight and so on. I don't see why or how the crypto code needs
to be fixed for something that is not related to it although it is affected.

The analogy would be like of the victims and a perpetrator. Lets take coresight
as an example for perpetrator and crypto as the victim here. Now we can try
to harden the protection and safeguard the victims which may or may not be
successful but it will be possible only if we know the victims beforehand.
If we just know one victim (lets say crypto code here), what happens to the
others which we haven't identified yet? Do we just wait for someone to write
an exploit based on this and then scramble to fix it?

Now the other foolproof way of saving the victims would be locking down the
perpetrator which would definitely save all the victims but that needs the
perpetrator to be identified. In our case, the perpetrator (coresight or any
other hw tracing) is already known, so we want to lock it down or restrict it
so that if there is actually a vulnerability in crypto or other areas, then
this HW assisted tracing wouldn't be able to help exploit it.

Initial change was to restrict this only to HW assisted instruction tracing [1]
but Peter wasn't convinced that this applies to only instruction tracing. Hence
this change for all kernel level pmu tracing. And this is a configurable option
via kernel config so that we don't force it on everyone. This config is also
required for coresight etm drivers because they have a sysfs mode of tracing as
well in addition to perf mode.

[1] https://lore.kernel.org/lkml/cover.1611909025.git.saiprakash.ranjan@codeaurora.org/

Thanks,
Sai
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

[Andi Kleen]

> The disk encryption is just one example and there might be others which
> we might not be aware of yet and we are not suspecting there is something
> wrong with the crypto code that needs to be fixed.

Then you don't have any leaks relating to branch tracing.

> restrict an external(in the sense that its not related to crypto or any
> other security related component) entity such as hardware assisted tracing
> like ARM coresight and so on. I don't see why or how the crypto code needs
> to be fixed for something that is not related to it although it is affected.

It's just a general property that if some code that is handling secrets
is data dependent it already leaks.


> The analogy would be like of the victims and a perpetrator. Lets take coresight
> as an example for perpetrator and crypto as the victim here. Now we can try

There's no victim with branch tracing, unless it is already leaky.

> If we just know one victim (lets say crypto code here), what happens to the
> others which we haven't identified yet? Do we just wait for someone to write
> an exploit based on this and then scramble to fix it?

For a useful security mitigation you need a threat model first I would say.

So you need to have at least some idea how an attack with branch
tracing would work.


> Initial change was to restrict this only to HW assisted instruction tracing [1]

I don't think it's needed for instruction tracing.

-Andi

Re: [PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

[Sai Prakash Ranjan]

Hi Andi,

On 2021-03-09 20:14, Andi Kleen wrote:
>> The disk encryption is just one example and there might be others 
>> which
>> we might not be aware of yet and we are not suspecting there is 
>> something
>> wrong with the crypto code that needs to be fixed.
> 
> Then you don't have any leaks relating to branch tracing.
> 
>> restrict an external(in the sense that its not related to crypto or 
>> any
>> other security related component) entity such as hardware assisted 
>> tracing
>> like ARM coresight and so on. I don't see why or how the crypto code 
>> needs
>> to be fixed for something that is not related to it although it is 
>> affected.
> 
> It's just a general property that if some code that is handling secrets
> is data dependent it already leaks.
> 
> 
>> The analogy would be like of the victims and a perpetrator. Lets take 
>> coresight
>> as an example for perpetrator and crypto as the victim here. Now we 
>> can try
> 
> There's no victim with branch tracing, unless it is already leaky.
> 
>> If we just know one victim (lets say crypto code here), what happens 
>> to the
>> others which we haven't identified yet? Do we just wait for someone to 
>> write
>> an exploit based on this and then scramble to fix it?
> 
> For a useful security mitigation you need a threat model first I would 
> say.
> 
> So you need to have at least some idea how an attack with branch
> tracing would work.
> 
> 
>> Initial change was to restrict this only to HW assisted instruction 
>> tracing [1]
> 
> I don't think it's needed for instruction tracing.
> 

 From what I know, newer ARM A-profile cores doesn't allow data tracing. 
And you
are saying that just the instruction tracing cannot be used to infer any
important data.

There are few security folks in CC who probably can give us more details 
on how
branch tracing can be used for an exploit. @mnissler?

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

[Mattias Nissler]

I know this reply is hopelessly late, but I still want to clarify a
few things (see inline for that) and provide some background on the
thinking that led to this proposal and where we ultimately landed for
the benefit of folks that come across this thread in the future.

The team working on tracing had asked me to provide input on the
security angle. We generally follow principle of least privilege /
attack surface minimization principles, and so the conclusion "kernel
tracing not needed" -> let's turn it off so we have one less thing to
potentially worry about was an easy one. What I hadn't been aware of
is that (a) no such config option exists in the kernel and (b) the
approach that the kernel is taking is to limit access to tracing data
to privileged users (via perf_event_paranoid sysctl, CAP_PERFMON).
Same for leaking kernel code pointers (note that kernel tracing
trivially defeats KASLR), privileged userspace can already read
/proc/kallsysms. While I would still like the kernel to give us a
build time option to disable kernel tracing, we can work with the
current state of affairs.

On Wed, Mar 10, 2021 at 4:17 PM Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> Hi Andi,
>
> On 2021-03-09 20:14, Andi Kleen wrote:
> >> The disk encryption is just one example

The intention behind that example was to illustrate the point that
there are some data items that the kernel should hide to all of
userspace, including privileged processes. I know this has been
controversial in the past, but I hope we all agree that the kernel (if
configured appropriately) is aiming to do so.

>
> and there might be others
> >> which
> >> we might not be aware of yet and we are not suspecting there is
> >> something
> >> wrong with the crypto code that needs to be fixed.
> >
> > Then you don't have any leaks relating to branch tracing.
>
> >
> >> restrict an external(in the sense that its not related to crypto or
> >> any
> >> other security related component) entity such as hardware assisted
> >> tracing
> >> like ARM coresight and so on. I don't see why or how the crypto code
> >> needs
> >> to be fixed for something that is not related to it although it is
> >> affected.
> >
> > It's just a general property that if some code that is handling secrets
> > is data dependent it already leaks.

Timing side channels have been a constant source of grief in crypto
implementations for decades now. Things have become better in the most
popular implementations, but bugs continue to be found. I happened to
chat about this topic with David Benjamin (boringssl maintainer)
recently, and his take was that timing side channels are a well
understood problem meanwhile, but in practice few implementations get
the details right. And the thing with high-resolution timestamps in
traces is that it gives you a tool to observe timing differences
closer to the source (so less noise) than if you just measure syscall
latency from userspace. So, in theory you are right - data-dependent
branches should not exist in crypto code, and if they do we should
just fix them, since they're potentially exploitable already. In
practice, timestamp information in tracing data can act as a
magnifying glass that may well make a difference between whether a
timing side channel is problematic in practice or not.

> >
> >
> >> The analogy would be like of the victims and a perpetrator. Lets take
> >> coresight
> >> as an example for perpetrator and crypto as the victim here. Now we
> >> can try
> >
> > There's no victim with branch tracing, unless it is already leaky.
> >
> >> If we just know one victim (lets say crypto code here), what happens
> >> to the
> >> others which we haven't identified yet? Do we just wait for someone to
> >> write
> >> an exploit based on this and then scramble to fix it?
> >
> > For a useful security mitigation you need a threat model first I would
> > say.
> >
> > So you need to have at least some idea how an attack with branch
> > tracing would work.
> >
> >
> >> Initial change was to restrict this only to HW assisted instruction
> >> tracing [1]
> >
> > I don't think it's needed for instruction tracing.
> >
>
>  From what I know, newer ARM A-profile cores doesn't allow data tracing.
> And you
> are saying that just the instruction tracing cannot be used to infer any
> important data.
>
> There are few security folks in CC who probably can give us more details
> on how
> branch tracing can be used for an exploit. @mnissler?
>
> Thanks,
> Sai
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member
> of Code Aurora Forum, hosted by The Linux Foundation