Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] coresight: configfs: restrict address parameter value to root
@ 2026-08-12  9:13 Junrui Luo via B4 Relay
  2026-08-12 14:12 ` Leo Yan
  0 siblings, 1 reply; 2+ messages in thread
From: Junrui Luo via B4 Relay @ 2026-08-12  9:13 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Leo Yan,
	Jonathan Corbet, Shuah Khan, Alexander Shishkin, Linu Cherian
  Cc: coresight, linux-arm-kernel, linux-doc, linux-kernel, Yuhao Jiang,
	stable, Junrui Luo

From: Junrui Luo <moonafterrain@outlook.com>

The preloaded 'gen_etrig' ETMv4 feature declares its only parameter as
{ .name = "address", .value = (u64)panic }, so on a relocatable kernel
the stored value is the post-KASLR runtime address of panic().

cscfg_param_value_show() prints that value verbatim with "0x%llx", and
CONFIGFS_ATTR() gives the attribute mode 0644 while every enclosing
directory is 0755. Once configfs is mounted, any local user reading
cs-syscfg/features/gen_etrig/params/address/value can recover the kernel
text base; neither kptr_restrict nor a capability check applies on that
path, and the plain u64 print bypasses the pointer-formatting
protections. The parameter exists even without trace hardware, since
cscfg_init() calls cscfg_preload() unconditionally and
coresight-cfg-pstop.o is linked into the core coresight module.

Mark parameters that can hold a kernel address, and give those a
config_item_type whose 'value' attribute is 0600. Parameters holding
plain numbers, such as the strobing 'window' and 'period' counts, keep
the existing mode.

Fixes: 4b7e62627a38 ("coresight: config: Add preloaded configuration")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Cc: stable@vger.kernel.org
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 Documentation/trace/coresight/coresight-config.rst |  5 +++++
 drivers/hwtracing/coresight/coresight-cfg-pstop.c  |  1 +
 drivers/hwtracing/coresight/coresight-config.h     |  3 +++
 .../coresight/coresight-syscfg-configfs.c          | 26 ++++++++++++++++++++++
 4 files changed, 35 insertions(+)

diff --git a/Documentation/trace/coresight/coresight-config.rst b/Documentation/trace/coresight/coresight-config.rst
index 6d5ffa6f7347..8df054b2aa10 100644
--- a/Documentation/trace/coresight/coresight-config.rst
+++ b/Documentation/trace/coresight/coresight-config.rst
@@ -202,6 +202,11 @@ Move to the params directory to examine and adjust parameters::
     # cat value
     0x3a98
 
+Updating a parameter requires root. Reading one does not, unless the parameter
+can hold a kernel address, in which case its 'value' is readable by root only.
+The preloaded 'gen_etrig' feature is such a case: its
+``features/gen_etrig/params/address/value`` defaults to the address of panic().
+
 Parameters adjusted in this way are reflected in all device instances that have
 loaded the feature.
 
diff --git a/drivers/hwtracing/coresight/coresight-cfg-pstop.c b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
index c2bfbd07bfaf..116954ad28b0 100644
--- a/drivers/hwtracing/coresight/coresight-cfg-pstop.c
+++ b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
@@ -19,6 +19,7 @@ static struct cscfg_parameter_desc gen_etrig_params[] = {
 	{
 		.name = "address",
 		.value = (u64)panic,
+		.sensitive = true,
 	},
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h
index 90fd937d3bd8..ead91c76fa52 100644
--- a/drivers/hwtracing/coresight/coresight-config.h
+++ b/drivers/hwtracing/coresight/coresight-config.h
@@ -46,10 +46,13 @@
  *
  * @name:  Name of parameter.
  * @value: Initial or default value.
+ * @sensitive: Value may be a kernel address, so restrict reads of it to
+ *	       callers permitted to see kernel pointers.
  */
 struct cscfg_parameter_desc {
 	const char *name;
 	u64 value;
+	bool sensitive;
 };
 
 /**
diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
index 2b40e556be87..2d6028c84c5e 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
+++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
@@ -304,16 +304,40 @@ static ssize_t cscfg_param_value_store(struct config_item *item,
 }
 CONFIGFS_ATTR(cscfg_param_, value);
 
+/*
+ * A parameter marked sensitive can hold a kernel address, so its value gets
+ * the same attribute with the world-readable bits dropped. Writing already
+ * required root. Open coded rather than CONFIGFS_ATTR_PERM(), as that macro
+ * derives the show/store names from the prefix and would need forwarders.
+ */
+static struct configfs_attribute cscfg_param_attr_value_sensitive = {
+	.ca_name	= "value",
+	.ca_mode	= 0600,
+	.ca_owner	= THIS_MODULE,
+	.show		= cscfg_param_value_show,
+	.store		= cscfg_param_value_store,
+};
+
 static struct configfs_attribute *cscfg_param_view_attrs[] = {
 	&cscfg_param_attr_value,
 	NULL,
 };
 
+static struct configfs_attribute *cscfg_param_sensitive_view_attrs[] = {
+	&cscfg_param_attr_value_sensitive,
+	NULL,
+};
+
 static const struct config_item_type cscfg_param_view_type = {
 	.ct_owner = THIS_MODULE,
 	.ct_attrs = cscfg_param_view_attrs,
 };
 
+static const struct config_item_type cscfg_param_sensitive_view_type = {
+	.ct_owner = THIS_MODULE,
+	.ct_attrs = cscfg_param_sensitive_view_attrs,
+};
+
 /*
  * configfs has far less functionality provided to add attributes dynamically than sysfs,
  * and the show and store fns pass the enclosing config_item so the actual attribute cannot
@@ -335,6 +359,8 @@ static int cscfg_create_params_group_items(struct cscfg_feature_desc *feat_desc,
 		param_item->param_idx = i;
 		config_group_init_type_name(&param_item->group,
 					    feat_desc->params_desc[i].name,
+					    feat_desc->params_desc[i].sensitive ?
+					    &cscfg_param_sensitive_view_type :
 					    &cscfg_param_view_type);
 		configfs_add_default_group(&param_item->group, params_group);
 	}

---
base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a
change-id: 20260812-coresight-fixes-9af0df331862

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>




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

* Re: [PATCH] coresight: configfs: restrict address parameter value to root
  2026-08-12  9:13 [PATCH] coresight: configfs: restrict address parameter value to root Junrui Luo via B4 Relay
@ 2026-08-12 14:12 ` Leo Yan
  0 siblings, 0 replies; 2+ messages in thread
From: Leo Yan @ 2026-08-12 14:12 UTC (permalink / raw)
  To: moonafterrain
  Cc: Suzuki K Poulose, Mike Leach, James Clark, Jonathan Corbet,
	Shuah Khan, Alexander Shishkin, Linu Cherian, coresight,
	linux-arm-kernel, linux-doc, linux-kernel, Yuhao Jiang, stable

On Wed, Aug 12, 2026 at 05:13:14PM +0800, Junrui Luo via B4 Relay wrote:

> Mark parameters that can hold a kernel address, and give those a
> config_item_type whose 'value' attribute is 0600. Parameters holding
> plain numbers, such as the strobing 'window' and 'period' counts, keep
> the existing mode.

Thanks for reporting the issue.

The patch seems overly complex to me. I'd suggest:

--- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
+++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
@@ -281,9 +281,16 @@ static ssize_t cscfg_param_value_show(struct config_item *item, char *page)
 {
        struct cscfg_fs_param *param_item = container_of(to_config_group(item),
                                                         struct cscfg_fs_param, group);
-       u64 value = param_item->feat_desc->params_desc[param_item->param_idx].value;
-
-       return scnprintf(page, PAGE_SIZE, "0x%llx\n", value);
+       struct cscfg_parameter_desc *param_desc =
+               param_item->feat_desc->params_desc + param_item->param_idx;
+       const char *name = param_desc->name;
+       u64 value = param_desc->value;
+
+       /* The kernel address should print with the "%pK" specifier */
+       if (!strncmp(name, "address"))
+               return scnprintf(page, PAGE_SIZE, "0x%pK\n", value);
+       else
+               return scnprintf(page, PAGE_SIZE, "0x%llx\n", value);
 }

We can add a flag (e.g., is_addr) in cscfg_parameter_desc to indicate
a parameter presents an address. Since currently only pstop's "address"
parameter has this issue, adding a general flag can be deferred until
it is actually needed.

Thanks,
Leo


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

end of thread, other threads:[~2026-08-12 14:13 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12  9:13 [PATCH] coresight: configfs: restrict address parameter value to root Junrui Luo via B4 Relay
2026-08-12 14:12 ` Leo Yan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox