[PATCH 0/3] Extend drm_ras for qaic telemetry

[PATCH 0/3] Extend drm_ras for qaic telemetry

[Zack McKevitt]

This patch series extends the drm_ras netlink interface to include a new
node type for telemetry nodes. Telemetry nodes support two operations,
reading and writing, and drivers may specify an optional predoit function
to validate each operation.

Telemetry read operations take the drm_ras node id and telemetry field to
be read and return the field's value and any status reported by the driver.
Telemetry write operations take the node id, telemetry field, and value,
and return only the status reported by the driver.

This extension removes the admin requirement on the list-nodes operation.
This allows drm_ras nodes to be discovered by userspace programs for drivers
that do not necessarily require admin privileges, like qaic.

The new drm_ras telemetry interface is implemented by the qaic driver, which
exposes the telemetry fields to userspace and enforces filesystem-esque
permission checks on telemetry operations in its telemetry_pre callback.

Zack McKevitt (3):
  drm/drm_ras: Remove admin-perm flag from list-nodes operation
  drm/drm_ras: Extend DRM RAS interface to include telemetry nodes
  accel/qaic: Add callbacks for DRM RAS netlink telemetry

 Documentation/netlink/specs/drm_ras.yaml |  56 ++-
 drivers/accel/qaic/Makefile              |   1 +
 drivers/accel/qaic/qaic.h                |  19 +
 drivers/accel/qaic/qaic_drv.c            |  19 +
 drivers/accel/qaic/qaic_telemetry.c      | 463 +++++++++++++++++++++++
 drivers/accel/qaic/qaic_telemetry.h      |  18 +
 drivers/gpu/drm/drm_ras.c                | 193 +++++++++-
 drivers/gpu/drm/drm_ras_nl.c             |  31 +-
 drivers/gpu/drm/drm_ras_nl.h             |   6 +
 include/drm/drm_ras.h                    |  39 ++
 include/uapi/drm/drm_ras.h               |  14 +
 include/uapi/drm/qaic_accel.h            |  28 ++
 12 files changed, 876 insertions(+), 11 deletions(-)
 create mode 100644 drivers/accel/qaic/qaic_telemetry.c
 create mode 100644 drivers/accel/qaic/qaic_telemetry.h

-- 
2.43.0

[PATCH 1/3] drm/drm_ras: Remove admin-perm flag from list-nodes operation

[Zack McKevitt]

Drivers may want to avoid requiring CAP_NET_ADMIN for subsequent
drm-ras operations. However, userspace must still be able to access
the list of RAS nodes to determine the node IDs. So, drop the
CAP_NET_ADMIN requirement for dumping RAS nodes so that drivers
with different permission requirements are still able to use the
interface.

This prepares the interface to be used for telemetry by the qaic
driver.

Signed-off-by: Zack McKevitt <zachary.mckevitt@oss.qualcomm.com>
---
 Documentation/netlink/specs/drm_ras.yaml | 1 -
 drivers/gpu/drm/drm_ras_nl.c             | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/netlink/specs/drm_ras.yaml b/Documentation/netlink/specs/drm_ras.yaml
index 4c2ac9a1ba3f..a88fdab7a1c7 100644
--- a/Documentation/netlink/specs/drm_ras.yaml
+++ b/Documentation/netlink/specs/drm_ras.yaml
@@ -112,7 +112,6 @@ operations:
            the node IDs. These IDs are required for all subsequent
            operations on nodes, such as querying error counters.
       attribute-set: node-attrs
-      flags: [admin-perm]
       dump:
         reply:
           attributes:
diff --git a/drivers/gpu/drm/drm_ras_nl.c b/drivers/gpu/drm/drm_ras_nl.c
index c9f5e0ceb3b5..ff0dc406c27c 100644
--- a/drivers/gpu/drm/drm_ras_nl.c
+++ b/drivers/gpu/drm/drm_ras_nl.c
@@ -46,7 +46,7 @@ static const struct genl_split_ops drm_ras_nl_ops[] = {
 	{
 		.cmd	= DRM_RAS_CMD_LIST_NODES,
 		.dumpit	= drm_ras_nl_list_nodes_dumpit,
-		.flags	= GENL_ADMIN_PERM | GENL_CMD_CAP_DUMP,
+		.flags	= GENL_CMD_CAP_DUMP,
 	},
 	{
 		.cmd		= DRM_RAS_CMD_GET_ERROR_COUNTER,
-- 
2.43.0

Re: [PATCH 1/3] drm/drm_ras: Remove admin-perm flag from list-nodes operation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] core: UAF in drm_ras_xa lookups during Netlink operations
- [High] core: uninitialized variable leak in Netlink dumpit handlers
- [Low] netlink: missing NLM_F_DUMP_INTR flag for lockless dump iteration
--

commit 46092107989be3447885b1e58b5d130104004fba
Author: Zack McKevitt <zachary.mckevitt@oss.qualcomm.com>

drm/drm_ras: Remove admin-perm flag from list-nodes operation

Drivers may want to avoid requiring CAP_NET_ADMIN for subsequent
drm-ras operations. So, drop the CAP_NET_ADMIN requirement for dumping
RAS nodes so that drivers with different permission requirements are
still able to use the interface.

> diff --git a/drivers/gpu/drm/drm_ras_nl.c b/drivers/gpu/drm/drm_ras_nl.c
> index c9f5e0ceb3b5f..ff0dc406c27c8 100644
> --- a/drivers/gpu/drm/drm_ras_nl.c
> +++ b/drivers/gpu/drm/drm_ras_nl.c
> @@ -46,7 +46,7 @@ static const struct genl_split_ops drm_ras_nl_ops[] = {
>  	{
>  		.cmd	= DRM_RAS_CMD_LIST_NODES,
>  		.dumpit	= drm_ras_nl_list_nodes_dumpit,
> -		.flags	= GENL_ADMIN_PERM | GENL_CMD_CAP_DUMP,
> +		.flags	= GENL_CMD_CAP_DUMP,

[Severity: Critical]
Does dropping the admin permission here safely handle the pointer management
in drm_ras_nl_list_nodes_dumpit() and get_node_error_counter()?

In drm_ras_nl_list_nodes_dumpit(), xa_for_each_start() iterates over the
xarray:

	xa_for_each_start(&drm_ras_xa, id, node, ctx->restart) {
		hdr = genlmsg_iput(skb, info);
		...
		ret = nla_put_string(skb, DRM_RAS_A_NODE_ATTRS_DEVICE_NAME,
				     node->device_name);

The xa_for_each_start() macro drops the RCU read lock before returning,
leaving the node pointer unprotected against concurrent freeing.

Could an unprivileged user concurrently querying the list while nodes are
unregistered trigger a use-after-free when node->device_name is read,
leading to memory corruption or data leaks?

A similar unprotected access occurs in get_node_error_counter():

	node = xa_load(&drm_ras_xa, node_id);
	if (!node || !node->query_error_counter)
		return -ENOENT;
	...
	return node->query_error_counter(node, error_id, name, value);

Does xa_load() protect the returned node pointer from being freed before the
query_error_counter() function pointer is dereferenced?

[Severity: High]
Can exposing drm_ras_nl_list_nodes_dumpit() to unprivileged users leak kernel
stack memory?

In drm_ras_nl_list_nodes_dumpit(), the ret variable is uninitialized:

	int ret;

	xa_for_each_start(&drm_ras_xa, id, node, ctx->restart) {
		...

If the drm_ras_xa xarray is empty, the loop will not execute, and the
function will return the uninitialized stack value. The same issue exists
in drm_ras_nl_get_error_counter_dumpit().

Additionally, if the loop does execute and the socket buffer is full, ret is
set to -EMSGSIZE and returned:

	if (ret == -EMSGSIZE)
		ctx->restart = id;

	return ret;

Does returning -EMSGSIZE break multi-part netlink dumps entirely by failing
to return skb->len when the buffer fills up?

[Severity: Low]
Is the NLM_F_DUMP_INTR flag required for this lockless dump iteration?

drm_ras_nl_list_nodes_dumpit() iterates over the lockless drm_ras_xa data
structure. If concurrent node registrations or unregistrations occur, the
iteration could skip or repeat items.

According to the Netlink subsystem rules, should such dumps maintain a
generation counter and set NLM_F_DUMP_INTR on the affected messages to
notify userspace of inconsistent states?

>  	},
>  	{
>  		.cmd		= DRM_RAS_CMD_GET_ERROR_COUNTER,

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009173152.2436195-1-zachary.mckevitt@oss.qualcomm.com?part=1

[PATCH 2/3] drm/drm_ras: Extend DRM RAS interface to include telemetry nodes

[Zack McKevitt]

Add a new node type, attribute set, and doit function for performing
telemetry read and write operations using the DRM RAS interface.

Signed-off-by: Zack McKevitt <zachary.mckevitt@oss.qualcomm.com>
---
 Documentation/netlink/specs/drm_ras.yaml |  55 ++++++-
 drivers/gpu/drm/drm_ras.c                | 193 ++++++++++++++++++++++-
 drivers/gpu/drm/drm_ras_nl.c             |  29 ++++
 drivers/gpu/drm/drm_ras_nl.h             |   6 +
 include/drm/drm_ras.h                    |  39 +++++
 include/uapi/drm/drm_ras.h               |  14 ++
 6 files changed, 327 insertions(+), 9 deletions(-)

diff --git a/Documentation/netlink/specs/drm_ras.yaml b/Documentation/netlink/specs/drm_ras.yaml
index a88fdab7a1c7..d558eb1cba3f 100644
--- a/Documentation/netlink/specs/drm_ras.yaml
+++ b/Documentation/netlink/specs/drm_ras.yaml
@@ -16,7 +16,7 @@ definitions:
     type: enum
     name: node-type
     value-start: 1
-    entries: [error-counter]
+    entries: [error-counter, telemetry]
     doc: >-
          Type of the node. Currently, only error-counter nodes are
          supported, which expose reliability counters for a hardware/software
@@ -100,6 +100,31 @@ attribute-sets:
         name: error-value
         type: u32
         doc: Current value of the error counter.
+  -
+    name: telemetry-attrs
+    attributes:
+      -
+        name: node-id
+        type: u32
+        doc: >-
+             Unique identifier for the node.
+             Assigned dynamically by the DRM RAS core upon registration.
+      -
+        name: field
+        type: u32
+        doc:  Telemetry field to be read/written, e.g., SoC temp.
+      -
+        name: value
+        type: u64
+        doc: Value of telemetry field that was read/written.
+      -
+        name: status
+        type: s32
+        doc: Status of the telemetry operation.
+      -
+        name: pad
+        type: pad
+        doc: Alignment padding used for 64-bit
 
 operations:
   list:
@@ -198,6 +223,34 @@ operations:
             - node-id
             - error-id
             - error-threshold
+    -
+      name: telemetry-read
+      doc: Perform a read of the specified telemetry field.
+      attribute-set: telemetry-attrs
+      do:
+        pre: drm-ras-nl-pre-telemetry
+        request:
+          attributes:
+            - node-id
+            - field
+        reply:
+          attributes:
+            - value
+            - status
+    -
+      name: telemetry-write
+      doc: Perform a write of the specified telemetry field.
+      attribute-set: telemetry-attrs
+      do:
+        pre: drm-ras-nl-pre-telemetry
+        request:
+          attributes:
+            - node-id
+            - field
+            - value
+        reply:
+          attributes:
+            - status
 
 mcast-groups:
   list:
diff --git a/drivers/gpu/drm/drm_ras.c b/drivers/gpu/drm/drm_ras.c
index b099eb67836e..b88293295cda 100644
--- a/drivers/gpu/drm/drm_ras.c
+++ b/drivers/gpu/drm/drm_ras.c
@@ -53,6 +53,15 @@
  * 7. SET_ERROR_THRESHOLD: Set error threshold of a given counter.
  *    Userspace must provide Node ID, Error ID and threshold to be set.
  *
+ * 8. TELEMETRY_READ: Read a telemetry value.
+ *    Userspace must provide Node ID and telemetry field to be read.
+ *    Returns the status of the read and the value read (if valid).
+ *
+ * 9. TELEMETRY_WRITE: Write a telemetry value.
+ *    Userspace must provide Node ID, the telemetry field to write, and the value
+ *    to be written.
+ *    Returns the status of the write.
+ *
  * Node registration:
  *
  * - drm_ras_node_register(): Registers a new node and assigns
@@ -63,7 +72,6 @@
  * Node type:
  *
  * - ERROR_COUNTER:
- *     + Currently, only error counters are supported.
  *     + The driver must implement the query_error_counter() callback to provide
  *       the name and the value of the error counter.
  *     + The driver must provide a error_counter_range.last value informing the
@@ -81,6 +89,12 @@
  *       that are raised by the hardware.
  *     + The driver is responsible for error threshold bounds checking.
  *
+ * - TELEMETRY:
+ *     + The driver must implement the telemetry_read() and telemetry_write()
+ *       callbacks.
+ *     + The driver can optionally provide a pre_doit callback, telemetry_pre(),
+ *       to enforce permissions checks on telemetry operations.
+ *
  * Netlink handlers:
  *
  * - drm_ras_nl_list_nodes_dumpit(): Implements the LIST_NODES
@@ -95,6 +109,12 @@
  *   operation, fetching the error threshold of a specific counter.
  * - drm_ras_nl_set_error_threshold_doit(): Implements the SET_ERROR_THRESHOLD doit
  *   operation, setting the error threshold of a specific counter.
+ * - drm_ras_nl_pre_telemetry(): Implements the TELEMETRY_READ and TELEMETRY_WRITE
+ *   pre_doit operation, returning 0 if valid.
+ * - drm_ras_nl_telemetry_read_doit(): Implements the TELEMETRY_READ doit operation,
+ *   invoking the driver's telemetry_read callback and returning the results.
+ * - drm_ras_nl_telemetry_write_doit(): Implements the TELEMETRY_WRITE doit operation,
+ *   invoking the driver's telemetry_write callback and returning the results.
  */
 
 static DEFINE_XARRAY_ALLOC(drm_ras_xa);
@@ -617,6 +637,156 @@ int drm_ras_nl_set_error_threshold_doit(struct sk_buff *skb, struct genl_info *i
 	return set_node_error_threshold(node_id, error_id, threshold);
 }
 
+/**
+ * drm_ras_nl_pre_telemetry - Enforce permission checks
+ * @ops: Generic netlink operations
+ * @skb: Netlink message buffer
+ * @info: Generic Netlink info containing attributes of the request
+ *
+ * Enforces a telemetry node's pre_doit conditions (if they exist).
+ *
+ * Returns 0 if no pre_doit is specified, otherwise returns the pre_doit.
+ */
+int drm_ras_nl_pre_telemetry(const struct genl_split_ops *ops, struct sk_buff *skb,
+			     struct genl_info *info)
+{
+	struct drm_ras_node *node;
+	u32 node_id;
+
+	node_id = nla_get_u32(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_NODE_ID]);
+	node = xa_load(&drm_ras_xa, node_id);
+
+	if (node->telemetry_pre)
+		return node->telemetry_pre(node);
+
+	return 0;
+}
+
+/**
+ * drm_ras_nl_telemetry_read_doit() - Query a telemetry value
+ * @skb: Netlink message buffer
+ * @info: Generic Netlink info containing attributes of the request
+ *
+ * Extracts the telemetry field from the netlink attributes and
+ * reads that value from the device.
+ *
+ * Return: 0 on success, or negative errno on failure.
+ */
+int drm_ras_nl_telemetry_read_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct drm_ras_node *node;
+	struct sk_buff *msg;
+	struct nlattr *hdr;
+	u32 node_id, field;
+	s32 status;
+	u64 value;
+	int ret;
+
+	if (!info->attrs || !info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_FIELD])
+		return -EINVAL;
+
+	field = nla_get_u32(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_FIELD]);
+	node_id = nla_get_u32(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_NODE_ID]);
+
+	node = xa_load(&drm_ras_xa, node_id);
+	if (!node || !node->telemetry_read)
+		return -ENOENT;
+
+	msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	hdr = genlmsg_iput(msg, info);
+	if (!hdr) {
+		nlmsg_free(msg);
+		return -EMSGSIZE;
+	}
+
+	ret = node->telemetry_read(node, &field, &value, &status);
+	if (ret) {
+		nlmsg_free(msg);
+		return ret;
+	}
+
+	ret = nla_put_s32(msg, DRM_RAS_A_TELEMETRY_ATTRS_STATUS, status);
+	if (ret) {
+		genlmsg_cancel(msg, hdr);
+		nlmsg_free(msg);
+		return ret;
+	}
+
+	ret = nla_put_64bit(msg, DRM_RAS_A_TELEMETRY_ATTRS_VALUE, sizeof(value),
+			    &value, DRM_RAS_A_TELEMETRY_ATTRS_PAD);
+	if (ret) {
+		genlmsg_cancel(msg, hdr);
+		nlmsg_free(msg);
+		return ret;
+	}
+
+	genlmsg_end(msg, hdr);
+
+	return genlmsg_reply(msg, info);
+}
+
+/**
+ * drm_ras_nl_telemetry_write_doit() - Write a telemetry value
+ * @skb: Netlink message buffer
+ * @info: Generic Netlink info containing attributes of the request
+ *
+ * Extracts the telemetry field and value to be written from the netlink
+ * attributes and writes that value to the specified field on the device.
+ *
+ * Return: 0 on success, or negative errno on failure.
+ */
+int drm_ras_nl_telemetry_write_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct drm_ras_node *node;
+	struct sk_buff *msg;
+	struct nlattr *hdr;
+	u32 node_id, field;
+	s32 status;
+	u64 value;
+	int ret;
+
+	if (!info->attrs || !info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_FIELD])
+		return -EINVAL;
+
+	value = nla_get_u64(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_VALUE]);
+	field = nla_get_u32(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_FIELD]);
+	node_id = nla_get_u32(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_NODE_ID]);
+
+	node = xa_load(&drm_ras_xa, node_id);
+	if (!node || !node->telemetry_read)
+		return -ENOENT;
+
+	msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	hdr = genlmsg_iput(msg, info);
+	if (!hdr) {
+		nlmsg_free(msg);
+		return -EMSGSIZE;
+	}
+
+	ret = node->telemetry_write(node, &field, &value, &status);
+	if (ret) {
+		nlmsg_free(msg);
+		return ret;
+	}
+
+	ret = nla_put_s32(msg, DRM_RAS_A_TELEMETRY_ATTRS_STATUS, status);
+	if (ret) {
+		genlmsg_cancel(msg, hdr);
+		nlmsg_free(msg);
+		return ret;
+	}
+
+	genlmsg_end(msg, hdr);
+
+	return genlmsg_reply(msg, info);
+}
+
 /**
  * drm_ras_node_register() - Register a new RAS node
  * @node: Node structure to register
@@ -631,14 +801,21 @@ int drm_ras_node_register(struct drm_ras_node *node)
 	if (!node->device_name || !node->node_name)
 		return -EINVAL;
 
-	/* Currently, only Error Counter Endpoints are supported */
-	if (node->type != DRM_RAS_NODE_TYPE_ERROR_COUNTER)
-		return -EINVAL;
-
-	/* Mandatory entries for Error Counter Node */
-	if (node->type == DRM_RAS_NODE_TYPE_ERROR_COUNTER &&
-	    (!node->error_counter_range.last || !node->query_error_counter))
+	/* Currently, only Error Counter and Telemetry endpoints are supported */
+	switch (node->type) {
+	case DRM_RAS_NODE_TYPE_ERROR_COUNTER:
+		/* Mandatory entries for Error Counter Node */
+		if (!node->error_counter_range.last || !node->query_error_counter)
+			return -EINVAL;
+		break;
+	case DRM_RAS_NODE_TYPE_TELEMETRY:
+		/* Mandatory entries for Telemetry Node */
+		if (!node->telemetry_read || !node->telemetry_write)
+			return -EINVAL;
+		break;
+	default:
 		return -EINVAL;
+	}
 
 	return xa_alloc(&drm_ras_xa, &node->id, node, xa_limit_32b, GFP_KERNEL);
 }
diff --git a/drivers/gpu/drm/drm_ras_nl.c b/drivers/gpu/drm/drm_ras_nl.c
index ff0dc406c27c..4577bc1a333e 100644
--- a/drivers/gpu/drm/drm_ras_nl.c
+++ b/drivers/gpu/drm/drm_ras_nl.c
@@ -41,6 +41,19 @@ static const struct nla_policy drm_ras_set_error_threshold_nl_policy[DRM_RAS_A_E
 	[DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_THRESHOLD] = { .type = NLA_U32, },
 };
 
+/* DRM_RAS_CMD_TELEMETRY_READ - do */
+static const struct nla_policy drm_ras_telemetry_read_nl_policy[DRM_RAS_A_TELEMETRY_ATTRS_FIELD + 1] = {
+	[DRM_RAS_A_TELEMETRY_ATTRS_NODE_ID] = { .type = NLA_U32, },
+	[DRM_RAS_A_TELEMETRY_ATTRS_FIELD] = { .type = NLA_U32, },
+};
+
+/* DRM_RAS_CMD_TELEMETRY_WRITE - do */
+static const struct nla_policy drm_ras_telemetry_write_nl_policy[DRM_RAS_A_TELEMETRY_ATTRS_VALUE + 1] = {
+	[DRM_RAS_A_TELEMETRY_ATTRS_NODE_ID] = { .type = NLA_U32, },
+	[DRM_RAS_A_TELEMETRY_ATTRS_FIELD] = { .type = NLA_U32, },
+	[DRM_RAS_A_TELEMETRY_ATTRS_VALUE] = { .type = NLA_U64, },
+};
+
 /* Ops table for drm_ras */
 static const struct genl_split_ops drm_ras_nl_ops[] = {
 	{
@@ -83,6 +96,22 @@ static const struct genl_split_ops drm_ras_nl_ops[] = {
 		.maxattr	= DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_THRESHOLD,
 		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
 	},
+	{
+		.cmd		= DRM_RAS_CMD_TELEMETRY_READ,
+		.pre_doit	= drm_ras_nl_pre_telemetry,
+		.doit		= drm_ras_nl_telemetry_read_doit,
+		.policy		= drm_ras_telemetry_read_nl_policy,
+		.maxattr	= DRM_RAS_A_TELEMETRY_ATTRS_FIELD,
+		.flags		= GENL_CMD_CAP_DO,
+	},
+	{
+		.cmd		= DRM_RAS_CMD_TELEMETRY_WRITE,
+		.pre_doit	= drm_ras_nl_pre_telemetry,
+		.doit		= drm_ras_nl_telemetry_write_doit,
+		.policy		= drm_ras_telemetry_write_nl_policy,
+		.maxattr	= DRM_RAS_A_TELEMETRY_ATTRS_VALUE,
+		.flags		= GENL_CMD_CAP_DO,
+	},
 };
 
 static const struct genl_multicast_group drm_ras_nl_mcgrps[] = {
diff --git a/drivers/gpu/drm/drm_ras_nl.h b/drivers/gpu/drm/drm_ras_nl.h
index 9aef097ea10f..4bdd7ed268a1 100644
--- a/drivers/gpu/drm/drm_ras_nl.h
+++ b/drivers/gpu/drm/drm_ras_nl.h
@@ -12,6 +12,9 @@
 
 #include <uapi/drm/drm_ras.h>
 
+int drm_ras_nl_pre_telemetry(const struct genl_split_ops *ops,
+			     struct sk_buff *skb, struct genl_info *info);
+
 int drm_ras_nl_list_nodes_dumpit(struct sk_buff *skb,
 				 struct netlink_callback *cb);
 int drm_ras_nl_get_error_counter_doit(struct sk_buff *skb,
@@ -24,6 +27,9 @@ int drm_ras_nl_get_error_threshold_doit(struct sk_buff *skb,
 					struct genl_info *info);
 int drm_ras_nl_set_error_threshold_doit(struct sk_buff *skb,
 					struct genl_info *info);
+int drm_ras_nl_telemetry_read_doit(struct sk_buff *skb, struct genl_info *info);
+int drm_ras_nl_telemetry_write_doit(struct sk_buff *skb,
+				    struct genl_info *info);
 
 enum {
 	DRM_RAS_NLGRP_ERROR_REPORT,
diff --git a/include/drm/drm_ras.h b/include/drm/drm_ras.h
index 1765a3130b09..2a2cdd44839d 100644
--- a/include/drm/drm_ras.h
+++ b/include/drm/drm_ras.h
@@ -100,6 +100,45 @@ struct drm_ras_node {
 	 */
 	int (*set_error_threshold)(struct drm_ras_node *node, u32 error_id, u32 threshold);
 
+	/**
+	 * @telemetry_pre:
+	 *
+	 * This callback is used to enforce any pre_doit conditions necessary
+	 * for telemetry. This is an optional callback for telemetry_node.
+	 *
+	 * Returns 0 on success, negative error code on failure.
+	 */
+	int (*telemetry_pre)(struct drm_ras_node *ep);
+
+	/**
+	 * @telemetry_read:
+	 *
+	 * This callback is used for a drm-ras telemetry node to read a
+	 * telemetry value. This is a mandatory callback for telemetry_node.
+	 *
+	 * The callback will read the desired device-specific field into value
+	 * and update status corresponding to a device-specific error code.
+	 *
+	 * Returns 0 on success, negative error code on failure.
+	 */
+	int (*telemetry_read)(struct drm_ras_node *ep, u32 *field, u64 *value,
+			      s32 *status);
+
+	/**
+	 * @telemetry_write:
+	 *
+	 * This callback is used for a drm-ras telemetry node to write a
+	 * telemetry value. This is a mandatory callback for telemetry_node.
+	 *
+	 * The callback will write the value to the desired device-specific
+	 * field and update status corresponding to a device-specific error
+	 * code.
+	 *
+	 * Returns 0 on success, negative error code on failure.
+	 */
+	int (*telemetry_write)(struct drm_ras_node *ep, u32 *field, u64 *value,
+			       s32 *status);
+
 	/** @priv: Driver private data */
 	void *priv;
 };
diff --git a/include/uapi/drm/drm_ras.h b/include/uapi/drm/drm_ras.h
index 2f832275ee6e..d39c78a9ed5a 100644
--- a/include/uapi/drm/drm_ras.h
+++ b/include/uapi/drm/drm_ras.h
@@ -16,6 +16,7 @@
  */
 enum drm_ras_node_type {
 	DRM_RAS_NODE_TYPE_ERROR_COUNTER = 1,
+	DRM_RAS_NODE_TYPE_TELEMETRY,
 };
 
 enum {
@@ -51,6 +52,17 @@ enum {
 	DRM_RAS_A_ERROR_EVENT_ATTRS_MAX = (__DRM_RAS_A_ERROR_EVENT_ATTRS_MAX - 1)
 };
 
+enum {
+	DRM_RAS_A_TELEMETRY_ATTRS_NODE_ID = 1,
+	DRM_RAS_A_TELEMETRY_ATTRS_FIELD,
+	DRM_RAS_A_TELEMETRY_ATTRS_VALUE,
+	DRM_RAS_A_TELEMETRY_ATTRS_STATUS,
+	DRM_RAS_A_TELEMETRY_ATTRS_PAD,
+
+	__DRM_RAS_A_TELEMETRY_ATTRS_MAX,
+	DRM_RAS_A_TELEMETRY_ATTRS_MAX = (__DRM_RAS_A_TELEMETRY_ATTRS_MAX - 1)
+};
+
 enum {
 	DRM_RAS_CMD_LIST_NODES = 1,
 	DRM_RAS_CMD_GET_ERROR_COUNTER,
@@ -58,6 +70,8 @@ enum {
 	DRM_RAS_CMD_ERROR_EVENT,
 	DRM_RAS_CMD_GET_ERROR_THRESHOLD,
 	DRM_RAS_CMD_SET_ERROR_THRESHOLD,
+	DRM_RAS_CMD_TELEMETRY_READ,
+	DRM_RAS_CMD_TELEMETRY_WRITE,
 
 	__DRM_RAS_CMD_MAX,
 	DRM_RAS_CMD_MAX = (__DRM_RAS_CMD_MAX - 1)
-- 
2.43.0

Re: [PATCH 2/3] drm/drm_ras: Extend DRM RAS interface to include telemetry nodes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] drm: NULL pointer dereference in drm_ras_nl_pre_telemetry() on invalid node
- [High] drm: missing netlink attribute presence checks cause NULL pointer dereferences
- [High] drm: Use-After-Free of drm_ras_node in netlink telemetry handlers
- [High] drm: DRM_RAS_CMD_TELEMETRY_WRITE lacks GENL_ADMIN_PERM capability
- [Medium] drm: uninitialized stack variables leaked in drm_ras_nl_telemetry_read_doit()
- [Low] drm_ras: drm_ras_nl_telemetry_write_doit validates wrong callback
- [Low] netlink: use of u64 and pad attributes instead of uint for new uAPI
--

commit 6e531a5b48e418d2f90c14c196ea1ac73f7170e4
Author: Zack McKevitt <zachary.mckevitt@oss.qualcomm.com>

drm/drm_ras: Extend DRM RAS interface to include telemetry nodes

This commit adds a new node type, attribute set, and doit function for
performing telemetry read and write operations using the DRM RAS interface.

> diff --git a/Documentation/netlink/specs/drm_ras.yaml b/Documentation/netlink/specs/drm_ras.yaml
> index a88fdab7a1c74..d558eb1cba3f1 100644
> --- a/Documentation/netlink/specs/drm_ras.yaml
> +++ b/Documentation/netlink/specs/drm_ras.yaml
[ ... ]
> @@ -100,6 +100,31 @@ attribute-sets:
>          name: error-value
>          type: u32
>          doc: Current value of the error counter.
> +  -
> +    name: telemetry-attrs
> +    attributes:
[ ... ]
> +      -
> +        name: value
> +        type: u64
> +        doc: Value of telemetry field that was read/written.
> +      -
> +        name: status
> +        type: s32
> +        doc: Status of the telemetry operation.
> +      -
> +        name: pad
> +        type: pad
> +        doc: Alignment padding used for 64-bit

[Severity: Low]
Does this new netlink family uAPI violate the generic netlink design
guidelines?

The telemetry-attrs set defines value as a fixed-width u64 and uses a pad
attribute. The generic netlink documentation explicitly prefers
variable-width 64-bit types (uint/sint) for new attributes to avoid padding
and 64-bit alignment issues on 32-bit architectures.

Since uAPI is immutable once merged, should this use uint natively instead
of introducing fixed-width structures requiring pad attributes?

> diff --git a/drivers/gpu/drm/drm_ras.c b/drivers/gpu/drm/drm_ras.c
> index b099eb67836e7..b88293295cda9 100644
> --- a/drivers/gpu/drm/drm_ras.c
> +++ b/drivers/gpu/drm/drm_ras.c
[ ... ]
> +int drm_ras_nl_pre_telemetry(const struct genl_split_ops *ops, struct sk_buff *skb,
> +			     struct genl_info *info)
> +{
> +	struct drm_ras_node *node;
> +	u32 node_id;
> +
> +	node_id = nla_get_u32(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_NODE_ID]);

[Severity: High]
Can this cause a NULL pointer dereference if userspace sends a malformed
message?

Generic netlink policies enforce the type of an attribute only if it
is present, but they do not enforce presence. If the
DRM_RAS_A_TELEMETRY_ATTRS_NODE_ID attribute is missing from the request,
info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_NODE_ID] will be NULL.

Passing NULL to nla_get_u32() dereferences NULL + NLA_HDRLEN, leading to an
immediate kernel oops.

> +	node = xa_load(&drm_ras_xa, node_id);
> +
> +	if (node->telemetry_pre)

[Severity: High]
Could this dereference a NULL pointer if the provided node_id does not
exist?

The xa_load() function returns NULL if the ID is not registered, and
this code immediately evaluates node->telemetry_pre without verifying if
node is a valid pointer.

> +		return node->telemetry_pre(node);
> +
> +	return 0;
> +}
[ ... ]
> +int drm_ras_nl_telemetry_read_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct drm_ras_node *node;
> +	struct sk_buff *msg;
> +	struct nlattr *hdr;
> +	u32 node_id, field;
> +	s32 status;
> +	u64 value;

[Severity: Medium]
Are these variables guaranteed to be fully initialized by the driver
callback?

They are declared on the kernel stack and passed by reference to the
node->telemetry_read() callback below.

> +	int ret;
> +
> +	if (!info->attrs || !info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_FIELD])
> +		return -EINVAL;
> +
> +	field = nla_get_u32(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_FIELD]);
> +	node_id = nla_get_u32(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_NODE_ID]);
> +
> +	node = xa_load(&drm_ras_xa, node_id);
> +	if (!node || !node->telemetry_read)
> +		return -ENOENT;

[Severity: High]
Does this lockless usage of the node pointer risk a use-after-free?

The pointer is obtained via xa_load(), which uses RCU internally but drops
the read lock before returning. The pointer is then used locklessly without
a reference count across the node->telemetry_read() callback.

Since telemetry_read() is allowed to sleep, could a concurrent hardware
unbind or module unload unregister and free the node while this callback is
still executing?

> +
> +	msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	hdr = genlmsg_iput(msg, info);
> +	if (!hdr) {
> +		nlmsg_free(msg);
> +		return -EMSGSIZE;
> +	}
> +
> +	ret = node->telemetry_read(node, &field, &value, &status);
> +	if (ret) {
> +		nlmsg_free(msg);
> +		return ret;
> +	}
> +
> +	ret = nla_put_s32(msg, DRM_RAS_A_TELEMETRY_ATTRS_STATUS, status);
> +	if (ret) {
> +		genlmsg_cancel(msg, hdr);
> +		nlmsg_free(msg);
> +		return ret;
> +	}
> +
> +	ret = nla_put_64bit(msg, DRM_RAS_A_TELEMETRY_ATTRS_VALUE, sizeof(value),
> +			    &value, DRM_RAS_A_TELEMETRY_ATTRS_PAD);

[Severity: Medium]
If the driver's telemetry_read() callback returns success (0) but fails to
explicitly populate the value or status fields, this writes uninitialized
kernel stack memory directly into the netlink reply.

Should these variables be initialized to zero when declared?

> +	if (ret) {
> +		genlmsg_cancel(msg, hdr);
> +		nlmsg_free(msg);
> +		return ret;
> +	}
[ ... ]
> +int drm_ras_nl_telemetry_write_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct drm_ras_node *node;
> +	struct sk_buff *msg;
> +	struct nlattr *hdr;
> +	u32 node_id, field;
> +	s32 status;
> +	u64 value;
> +	int ret;
> +
> +	if (!info->attrs || !info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_FIELD])
> +		return -EINVAL;
> +
> +	value = nla_get_u64(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_VALUE]);
> +	field = nla_get_u32(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_FIELD]);
> +	node_id = nla_get_u32(info->attrs[DRM_RAS_A_TELEMETRY_ATTRS_NODE_ID]);

[Severity: High]
Similar to the pre_doit handler, is it possible for a malformed message
missing the VALUE or NODE_ID attributes to trigger a NULL pointer
dereference here?

The presence of DRM_RAS_A_TELEMETRY_ATTRS_FIELD is explicitly checked above,
but VALUE and NODE_ID are extracted unconditionally.

> +
> +	node = xa_load(&drm_ras_xa, node_id);
> +	if (!node || !node->telemetry_read)

[Severity: Low]
Does this code validate the wrong callback?

The telemetry write handler checks for the presence of node->telemetry_read,
but later invokes node->telemetry_write(). While node registration might
currently ensure both are populated, checking the read callback here creates
a brittle dependency on the registration logic.

> +		return -ENOENT;
[ ... ]
> diff --git a/drivers/gpu/drm/drm_ras_nl.c b/drivers/gpu/drm/drm_ras_nl.c
> --- a/drivers/gpu/drm/drm_ras_nl.c
> +++ b/drivers/gpu/drm/drm_ras_nl.c
[ ... ]
> @@ -83,6 +96,22 @@ static const struct genl_split_ops drm_ras_nl_ops[] = {
[ ... ]
> +	{
> +		.cmd		= DRM_RAS_CMD_TELEMETRY_WRITE,
> +		.pre_doit	= drm_ras_nl_pre_telemetry,
> +		.doit		= drm_ras_nl_telemetry_write_doit,
> +		.policy		= drm_ras_telemetry_write_nl_policy,
> +		.maxattr	= DRM_RAS_A_TELEMETRY_ATTRS_VALUE,
> +		.flags		= GENL_CMD_CAP_DO,
> +	},
>  };

[Severity: High]
Should this command require the GENL_ADMIN_PERM capability?

Unlike DRM_RAS_CMD_SET_ERROR_THRESHOLD, the telemetry write command is missing
the GENL_ADMIN_PERM flag. This allows unprivileged users to issue arbitrary
writes to hardware components, potentially leading to denial of service or
hardware state corruption.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009173152.2436195-1-zachary.mckevitt@oss.qualcomm.com?part=2

[PATCH 3/3] accel/qaic: Add callbacks for DRM RAS netlink telemetry

[Zack McKevitt]

Use the DRM RAS netlink infrastructure to perform device telemetry.
During PCI probe, create a DRM RAS node with DRM_RAS_NODE_TYPE_TELEMETRY
type and add callbacks to telemetry_request for read and write operations.

Co-developed-by: Jeff Hugo <jeff.hugo@oss.qualcomm.com>
Signed-off-by: Jeff Hugo <jeff.hugo@oss.qualcomm.com>
Signed-off-by: Zack McKevitt <zachary.mckevitt@oss.qualcomm.com>
---
 drivers/accel/qaic/Makefile         |   1 +
 drivers/accel/qaic/qaic.h           |  19 ++
 drivers/accel/qaic/qaic_drv.c       |  19 ++
 drivers/accel/qaic/qaic_telemetry.c | 463 ++++++++++++++++++++++++++++
 drivers/accel/qaic/qaic_telemetry.h |  18 ++
 include/uapi/drm/qaic_accel.h       |  28 ++
 6 files changed, 548 insertions(+)
 create mode 100644 drivers/accel/qaic/qaic_telemetry.c
 create mode 100644 drivers/accel/qaic/qaic_telemetry.h

diff --git a/drivers/accel/qaic/Makefile b/drivers/accel/qaic/Makefile
index 71f727b74da3..b2bbe9b3e077 100644
--- a/drivers/accel/qaic/Makefile
+++ b/drivers/accel/qaic/Makefile
@@ -13,6 +13,7 @@ qaic-y := \
 	qaic_ras.o \
 	qaic_ssr.o \
 	qaic_sysfs.o \
+	qaic_telemetry.o \
 	qaic_timesync.o \
 	sahara.o
 
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 02ca99000e2a..f5dbeadba658 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -17,6 +17,7 @@
 #include <linux/workqueue.h>
 #include <drm/drm_device.h>
 #include <drm/drm_gem.h>
+#include <drm/drm_ras.h>
 
 #define QAIC_DBC_BASE		SZ_128K
 #define QAIC_DBC_SIZE		SZ_4K
@@ -203,6 +204,24 @@ struct qaic_device {
 	void			*ssr_mhi_buf;
 	/* DBC which is under SSR. Sentinel U32_MAX would mean that no SSR in progress */
 	u32			ssr_dbc;
+	/* MHI telemetry channel device */
+	struct mhi_device	*tele_ch;
+	/* Head in list of requests queued in MHI telemetry device */
+	struct list_head	tele_xfer_list;
+	/* Req. ID of request that will be queued next in MHI telemetry device */
+	u32			tele_next_seq_num;
+	/* Synchronizes MHI telemetry device transactions and its xfer list */
+	struct mutex		tele_mutex;
+	/*
+	 * TRUE: A tx MHI transaction has failed and a rx buffer is still queued
+	 * in telemetry device. Such a buffer is considered lost rx buffer
+	 * FALSE: No rx buffer is lost in telemetry device
+	 */
+	bool			tele_lost_buf;
+	/* Work queue for tasks related to MHI telemetry device */
+	struct workqueue_struct	*tele_wq;
+	/* Netlink telemetry node */
+	struct drm_ras_node *ras_node;
 	/* Array of DBC struct of this device */
 	struct dma_bridge_chan	dbc[] __counted_by(num_dbc);
 };
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index c1bb6db78d12..d2aed51052a6 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -32,6 +32,7 @@
 #include "qaic_debugfs.h"
 #include "qaic_ras.h"
 #include "qaic_ssr.h"
+#include "qaic_telemetry.h"
 #include "qaic_timesync.h"
 #include "sahara.h"
 
@@ -381,6 +382,7 @@ static void qaic_notify_reset(struct qaic_device *qdev)
 	qdev->dev_state = QAIC_OFFLINE;
 	/* wake up any waiters to avoid waiting for timeouts at sync */
 	wake_all_cntl(qdev);
+	wake_all_telemetry(qdev);
 	for (i = 0; i < qdev->num_dbc; ++i)
 		wakeup_dbc(qdev, i);
 	synchronize_srcu(&qdev->dev_lock);
@@ -431,6 +433,9 @@ static struct qaic_device *create_qdev(struct pci_dev *pdev,
 	if (ret)
 		return NULL;
 	ret = drmm_mutex_init(drm, &qdev->bootlog_mutex);
+	if (ret)
+		return NULL;
+	ret = drmm_mutex_init(drm, &qdev->tele_mutex);
 	if (ret)
 		return NULL;
 
@@ -443,6 +448,9 @@ static struct qaic_device *create_qdev(struct pci_dev *pdev,
 	qdev->ssr_wq = qaicm_wq_init(drm, "qaic_ssr");
 	if (IS_ERR(qdev->ssr_wq))
 		return NULL;
+	qdev->tele_wq = qaicm_wq_init(drm, "qaic_tele");
+	if (IS_ERR(qdev->tele_wq))
+		return NULL;
 
 	ret = qaicm_srcu_init(drm, &qdev->dev_lock);
 	if (ret)
@@ -458,6 +466,7 @@ static struct qaic_device *create_qdev(struct pci_dev *pdev,
 
 	INIT_LIST_HEAD(&qdev->cntl_xfer_list);
 	INIT_LIST_HEAD(&qdev->bootlog);
+	INIT_LIST_HEAD(&qdev->tele_xfer_list);
 	INIT_LIST_HEAD(&qddev->users);
 
 	for (i = 0; i < qdev->num_dbc; ++i) {
@@ -595,6 +604,10 @@ static int qaic_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (ret)
 		return ret;
 
+	ret = qaic_drm_ras_register_node(qdev);
+	if (ret)
+		return ret;
+
 	qdev->mhi_cntrl = qaic_mhi_register_controller(pdev, qdev->bar_mhi, mhi_irq,
 						       qdev->single_msi, config->family);
 	if (IS_ERR(qdev->mhi_cntrl)) {
@@ -615,6 +628,7 @@ static void qaic_pci_remove(struct pci_dev *pdev)
 
 	qaic_dev_reset_clean_local_state(qdev);
 	qaic_mhi_free_controller(qdev->mhi_cntrl, link_up);
+	qaic_drm_ras_unregister_node(qdev);
 	qaic_destroy_drm_device(qdev, QAIC_NO_PARTITION);
 }
 
@@ -813,6 +827,10 @@ static int __init qaic_init(void)
 		goto free_bootlog;
 	}
 
+	ret = qaic_telemetry_register();
+	if (ret)
+		pr_debug("qaic: qaic_telemetry_register failed %d\n", ret);
+
 	return 0;
 
 free_bootlog:
@@ -842,6 +860,7 @@ static void __exit qaic_exit(void)
 	 * reinitializing the link_up state after the cleanup is done.
 	 */
 	link_up = true;
+	qaic_telemetry_unregister();
 	qaic_ssr_unregister();
 	qaic_ras_unregister();
 	qaic_bootlog_unregister();
diff --git a/drivers/accel/qaic/qaic_telemetry.c b/drivers/accel/qaic/qaic_telemetry.c
new file mode 100644
index 000000000000..dbfb36cf319c
--- /dev/null
+++ b/drivers/accel/qaic/qaic_telemetry.c
@@ -0,0 +1,463 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. */
+/* Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. */
+
+#include <drm/drm_file.h>
+#include <drm/drm_ras.h>
+#include <asm/byteorder.h>
+#include <linux/completion.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/kref.h>
+#include <linux/list.h>
+#include <linux/mhi.h>
+#include <linux/mutex.h>
+#include <linux/namei.h>
+#include <linux/path.h>
+#include <linux/sprintf.h>
+#include <linux/srcu.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+#include <uapi/drm/qaic_accel.h>
+
+#include "qaic.h"
+#include "qaic_telemetry.h"
+
+#define MAGIC				0x55AA
+#define VERSION				0x1
+
+enum fw_errno {
+	ERR_READ_ONLY		= -1,
+	ERR_NOT_SUPPORTED	= -2,
+	ERR_WHILE_PROCESSING	= -3,
+	ERR_INVALID_VALUE	= -4,
+	ERR_INVALID_HEADER	= -5,
+};
+
+enum cmd_type {
+	TYPE_READ,  /* read value from device */
+	TYPE_WRITE, /* write value to device */
+};
+
+enum msg_type {
+	MSG_PUSH, /* async push from device */
+	MSG_REQ,  /* sync request to device */
+	MSG_RESP, /* sync response from device */
+};
+
+struct telemetry_data {
+	u8	cmd;
+	u8	cmd_type;
+	u8	status;
+	__le64	val; /* signed */
+} __packed;
+
+struct telemetry_header {
+	__le16	magic;
+	__le16	ver;
+	__le32	seq_num;
+	u8	type;
+	u8	id;
+	__le16	len;
+} __packed;
+
+struct telemetry_msg { /* little endian encoded */
+	struct telemetry_header hdr;
+	struct telemetry_data data;
+} __packed;
+
+struct wrapper_msg {
+	struct kref ref_count;
+	struct telemetry_msg msg;
+};
+
+struct xfer_queue_elem {
+	/*
+	 * Node in list of ongoing transfer request on telemetry channel.
+	 * Maintained by root device struct
+	 */
+	struct list_head list;
+	/* Sequence number of this transfer request */
+	u32 seq_num;
+	/* This is used to wait on until completion of transfer request */
+	struct completion xfer_done;
+	/* Received data from device */
+	void *buf;
+};
+
+struct resp_work {
+	/* Work struct to schedule work coming on QAIC_TELEMETRY channel */
+	struct work_struct work;
+	/* Root struct of device, used to access device resources */
+	struct qaic_device *qdev;
+	/* Buffer used by MHI for transfer requests */
+	void *buf;
+};
+
+static void free_wrapper(struct kref *ref)
+{
+	struct wrapper_msg *wrapper = container_of(ref, struct wrapper_msg, ref_count);
+
+	kfree(wrapper);
+}
+
+int telemetry_request(struct qaic_device *qdev, u8 cmd, u8 cmd_type, s64 *val)
+{
+	struct wrapper_msg *wrapper;
+	struct xfer_queue_elem elem;
+	struct telemetry_msg *resp;
+	struct telemetry_msg *req;
+	long ret = 0;
+
+	wrapper = kzalloc_obj(*wrapper);
+	if (!wrapper)
+		return -ENOMEM;
+
+	kref_init(&wrapper->ref_count);
+	req = &wrapper->msg;
+
+	ret = mutex_lock_interruptible(&qdev->tele_mutex);
+	if (ret)
+		goto free_req;
+
+	req->hdr.magic = cpu_to_le16(MAGIC);
+	req->hdr.ver = cpu_to_le16(VERSION);
+	req->hdr.seq_num = cpu_to_le32(qdev->tele_next_seq_num++);
+	req->hdr.type = MSG_REQ;
+	req->hdr.id = 0;
+	req->hdr.len = cpu_to_le16(sizeof(req->data));
+
+	req->data.cmd = cmd;
+	req->data.cmd_type = cmd_type;
+	req->data.status = 0;
+	if (cmd_type == TYPE_READ)
+		req->data.val = cpu_to_le64(0);
+	else
+		req->data.val = cpu_to_le64(*val);
+
+	elem.seq_num = qdev->tele_next_seq_num - 1;
+	elem.buf = NULL;
+	init_completion(&elem.xfer_done);
+	if (likely(!qdev->tele_lost_buf)) {
+		resp = kzalloc_obj(*resp);
+		if (!resp) {
+			mutex_unlock(&qdev->tele_mutex);
+			ret = -ENOMEM;
+			goto free_req;
+		}
+
+		ret = mhi_queue_buf(qdev->tele_ch, DMA_FROM_DEVICE, resp, sizeof(*resp), MHI_EOT);
+		if (ret) {
+			mutex_unlock(&qdev->tele_mutex);
+			goto free_resp;
+		}
+	} else {
+		/*
+		 * we lost a buffer because we queued a recv buf, but then
+		 * queuing the corresponding tx buf failed. To try to avoid
+		 * a memory leak, lets reclaim it and use it for this
+		 * transaction.
+		 */
+		qdev->tele_lost_buf = false;
+	}
+
+	kref_get(&wrapper->ref_count);
+	ret = mhi_queue_buf(qdev->tele_ch, DMA_TO_DEVICE, req, sizeof(*req), MHI_EOT);
+	if (ret) {
+		qdev->tele_lost_buf = true;
+		kref_put(&wrapper->ref_count, free_wrapper);
+		mutex_unlock(&qdev->tele_mutex);
+		goto free_req;
+	}
+
+	list_add_tail(&elem.list, &qdev->tele_xfer_list);
+	mutex_unlock(&qdev->tele_mutex);
+
+	ret = wait_for_completion_interruptible_timeout(&elem.xfer_done, HZ);
+	/*
+	 * not using _interruptable because we have to cleanup or we'll
+	 * likely cause memory corruption
+	 */
+	mutex_lock(&qdev->tele_mutex);
+	if (!list_empty(&elem.list))
+		list_del(&elem.list);
+	if (!ret && !elem.buf)
+		ret = -ETIMEDOUT;
+	else if (ret > 0 && !elem.buf)
+		ret = -EIO;
+	mutex_unlock(&qdev->tele_mutex);
+
+	resp = elem.buf;
+
+	if (ret < 0)
+		goto free_resp;
+
+	if (le16_to_cpu(resp->hdr.magic) != MAGIC || le16_to_cpu(resp->hdr.ver) != VERSION ||
+	    resp->hdr.type != MSG_RESP || resp->hdr.id != 0 ||
+	    le16_to_cpu(resp->hdr.len) != sizeof(resp->data) || resp->data.cmd != cmd ||
+	    resp->data.cmd_type != cmd_type) {
+		ret = -EINVAL;
+		goto free_resp;
+	}
+
+	if (resp->data.status) {
+		switch ((int8_t) resp->data.status) {
+		case ERR_READ_ONLY:
+			ret = -EROFS;
+			break;
+		case ERR_NOT_SUPPORTED:
+			ret = -EOPNOTSUPP;
+			break;
+		case ERR_WHILE_PROCESSING:
+			ret = -EREMOTEIO;
+			break;
+		case ERR_INVALID_VALUE:
+			ret = -EINVAL;
+			break;
+		case ERR_INVALID_HEADER:
+			ret = -EBADR;
+			break;
+		default:
+			ret = -EPROTO;
+		}
+		goto free_resp;
+	}
+
+	if (cmd_type == TYPE_READ)
+		*val = le64_to_cpu(resp->data.val);
+
+	ret = 0;
+
+free_resp:
+	kfree(resp);
+free_req:
+	kref_put(&wrapper->ref_count, free_wrapper);
+
+	return ret;
+}
+
+static int qaic_telemetry_mhi_probe(struct mhi_device *mhi_dev, const struct mhi_device_id *id)
+{
+	struct qaic_device *qdev = pci_get_drvdata(to_pci_dev(mhi_dev->mhi_cntrl->cntrl_dev));
+	int ret;
+
+	ret = mhi_prepare_for_transfer(mhi_dev);
+	if (ret)
+		return ret;
+
+	dev_set_drvdata(&mhi_dev->dev, qdev);
+	qdev->tele_ch = mhi_dev;
+	qdev->tele_lost_buf = false;
+
+	return 0;
+}
+
+static void qaic_telemetry_mhi_remove(struct mhi_device *mhi_dev)
+{
+	struct qaic_device *qdev = dev_get_drvdata(&mhi_dev->dev);
+
+	mhi_unprepare_from_transfer(qdev->tele_ch);
+	qdev->tele_ch = NULL;
+}
+
+static void resp_worker(struct work_struct *work)
+{
+	struct resp_work *resp = container_of(work, struct resp_work, work);
+	struct qaic_device *qdev = resp->qdev;
+	struct telemetry_msg *msg = resp->buf;
+	struct xfer_queue_elem *elem;
+	struct xfer_queue_elem *i;
+	bool found = false;
+
+	if (le16_to_cpu(msg->hdr.magic) != MAGIC) {
+		kfree(msg);
+		kfree(resp);
+		return;
+	}
+
+	mutex_lock(&qdev->tele_mutex);
+	list_for_each_entry_safe(elem, i, &qdev->tele_xfer_list, list) {
+		if (elem->seq_num == le32_to_cpu(msg->hdr.seq_num)) {
+			found = true;
+			list_del_init(&elem->list);
+			elem->buf = msg;
+			complete_all(&elem->xfer_done);
+			break;
+		}
+	}
+	mutex_unlock(&qdev->tele_mutex);
+
+	if (!found)
+		/* request must have timed out, drop packet */
+		kfree(msg);
+
+	kfree(resp);
+}
+
+static void qaic_telemetry_mhi_ul_xfer_cb(struct mhi_device *mhi_dev,
+					  struct mhi_result *mhi_result)
+{
+	struct telemetry_msg *msg = mhi_result->buf_addr;
+	struct wrapper_msg *wrapper;
+
+	wrapper = container_of(msg, struct wrapper_msg, msg);
+	kref_put(&wrapper->ref_count, free_wrapper);
+}
+
+static void qaic_telemetry_mhi_dl_xfer_cb(struct mhi_device *mhi_dev,
+					  struct mhi_result *mhi_result)
+{
+	struct qaic_device *qdev = dev_get_drvdata(&mhi_dev->dev);
+	struct telemetry_msg *msg = mhi_result->buf_addr;
+	struct resp_work *resp;
+
+	if (mhi_result->transaction_status) {
+		kfree(msg);
+		return;
+	}
+
+	resp = kmalloc(sizeof(*resp), GFP_ATOMIC);
+	if (!resp) {
+		pci_err(qdev->pdev, "dl_xfer_cb alloc fail, dropping message\n");
+		kfree(msg);
+		return;
+	}
+
+	INIT_WORK(&resp->work, resp_worker);
+	resp->qdev = qdev;
+	resp->buf = msg;
+	queue_work(qdev->tele_wq, &resp->work);
+}
+
+static const struct mhi_device_id qaic_telemetry_mhi_match_table[] = {
+	{ .chan = "QAIC_TELEMETRY", },
+	{},
+};
+
+static struct mhi_driver qaic_telemetry_mhi_driver = {
+	.id_table = qaic_telemetry_mhi_match_table,
+	.remove = qaic_telemetry_mhi_remove,
+	.probe = qaic_telemetry_mhi_probe,
+	.ul_xfer_cb = qaic_telemetry_mhi_ul_xfer_cb,
+	.dl_xfer_cb = qaic_telemetry_mhi_dl_xfer_cb,
+	.driver = {
+		.name = "qaic_telemetry",
+	},
+};
+
+int qaic_telemetry_register(void)
+{
+	return mhi_driver_register(&qaic_telemetry_mhi_driver);
+}
+
+void qaic_telemetry_unregister(void)
+{
+	mhi_driver_unregister(&qaic_telemetry_mhi_driver);
+}
+
+void wake_all_telemetry(struct qaic_device *qdev)
+{
+	struct xfer_queue_elem *elem;
+	struct xfer_queue_elem *i;
+
+	mutex_lock(&qdev->tele_mutex);
+	list_for_each_entry_safe(elem, i, &qdev->tele_xfer_list, list) {
+		list_del_init(&elem->list);
+		complete_all(&elem->xfer_done);
+	}
+	qdev->tele_lost_buf = false;
+	mutex_unlock(&qdev->tele_mutex);
+}
+
+static int qaic_telemetry_pre(struct drm_ras_node *ep)
+{
+	struct qaic_device *qdev = ep->priv;
+	struct inode *dev_inode;
+	struct path path;
+	char devpath[24];
+	int ret;
+
+	snprintf(devpath, sizeof(devpath), "/dev/accel/%s", to_accel_kdev(qdev->qddev)->kobj.name);
+	ret = kern_path(devpath, LOOKUP_FOLLOW, &path);
+	if (ret)
+		return -EINVAL;
+
+	dev_inode = d_inode(path.dentry);
+	ret = inode_permission(&nop_mnt_idmap, dev_inode, MAY_READ | MAY_WRITE);
+	path_put(&path);
+
+	return ret;
+}
+
+static int qaic_telemetry_read(struct drm_ras_node *ep, u32 *field, u64 *value, s32 *status)
+{
+	struct qaic_device *qdev = ep->priv;
+
+	*status = (s32)telemetry_request(qdev, *field, TYPE_READ, value);
+
+	return *status;
+}
+
+static int qaic_telemetry_write(struct drm_ras_node *ep, u32 *field, u64 *value, s32 *status)
+{
+	struct qaic_device *qdev = ep->priv;
+
+	*status = (s32)telemetry_request(qdev, *field, TYPE_WRITE, value);
+
+	return *status;
+}
+
+int qaic_drm_ras_register_node(struct qaic_device *qdev)
+{
+	struct drm_ras_node *node;
+	struct pci_dev *pdev = qdev->pdev;
+	const char *device_name, *node_name;
+	int ret;
+
+	node = kzalloc_obj(struct drm_ras_node);
+	if (!node)
+		return -ENOMEM;
+
+	device_name = kasprintf(GFP_KERNEL, "%04x:%02x:%02x.%d",
+				pci_domain_nr(pdev->bus), pdev->bus->number,
+				PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+	if (!device_name) {
+		kfree(node);
+		return -ENOMEM;
+	}
+
+	node_name = kasprintf(GFP_KERNEL, "qaic_telemetry");
+	if (!node_name) {
+		kfree(device_name);
+		kfree(node);
+		return -ENOMEM;
+	}
+
+	node->device_name = device_name;
+	node->node_name = node_name;
+	node->type = DRM_RAS_NODE_TYPE_TELEMETRY;
+	node->priv = qdev;
+	node->telemetry_read = qaic_telemetry_read;
+	node->telemetry_write = qaic_telemetry_write;
+	node->telemetry_pre = qaic_telemetry_pre;
+
+	ret = drm_ras_node_register(node);
+	if (ret) {
+		kfree(device_name);
+		kfree(node_name);
+		kfree(qdev->ras_node);
+		return ret;
+	}
+
+	qdev->ras_node = node;
+	return 0;
+}
+
+void qaic_drm_ras_unregister_node(struct qaic_device *qdev)
+{
+	drm_ras_node_unregister(qdev->ras_node);
+	kfree(qdev->ras_node->node_name);
+	kfree(qdev->ras_node->device_name);
+	kfree(qdev->ras_node);
+}
diff --git a/drivers/accel/qaic/qaic_telemetry.h b/drivers/accel/qaic/qaic_telemetry.h
new file mode 100644
index 000000000000..a80922591e37
--- /dev/null
+++ b/drivers/accel/qaic/qaic_telemetry.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+/* Copyright (c) 2020, The Linux Foundation. All rights reserved. */
+
+#ifndef __QAIC_TELEMETRY_H__
+#define __QAIC_TELEMETRY_H__
+
+#include "qaic.h"
+
+int telemetry_request(struct qaic_device *qdev, u8 cmd, u8 cmd_type, s64 *val);
+int qaic_drm_ras_register_node(struct qaic_device *qdev);
+void qaic_drm_ras_unregister_node(struct qaic_device *qdev);
+
+int qaic_telemetry_register(void);
+void qaic_telemetry_unregister(void);
+void wake_all_telemetry(struct qaic_device *qdev);
+
+#endif /* __QAIC_TELEMETRY_H__ */
diff --git a/include/uapi/drm/qaic_accel.h b/include/uapi/drm/qaic_accel.h
index c92d0309d583..5aac05101426 100644
--- a/include/uapi/drm/qaic_accel.h
+++ b/include/uapi/drm/qaic_accel.h
@@ -51,6 +51,34 @@ extern "C" {
 #define QAIC_TRANS_VALIDATE_PARTITION_FROM_DEV	19
 #define QAIC_TRANS_VALIDATE_PARTITION_TO_DEV	20
 
+/* Telemetry fields */
+#define QAIC_DEVINFO_THERMAL_SOC_MAX_TEMP	1
+#define QAIC_DEVINFO_THERMAL_BOARD_TEMP		2
+#define QAIC_DEVINFO_THERMAL_BOARD_MAX_TEMP	3
+#define QAIC_DEVINFO_THERMAL_DDR_TEMP		4
+#define QAIC_DEVINFO_THERMAL_WARNING_TEMP	5
+#define QAIC_DEVINFO_THERMAL_SHUTDOWN_TEMP	6
+#define QAIC_DEVINFO_BOARD_TDP			7
+#define QAIC_DEVINFO_BOARD_POWER		8
+#define QAIC_DEVINFO_POWER_STATE		9
+#define QAIC_DEVINFO_POWER_MAX			10
+#define QAIC_DEVINFO_THROTTLE_PERCENT		11
+#define QAIC_DEVINFO_THROTTLE_TIME		12
+#define QAIC_DEVINFO_UPTIME			13
+#define QAIC_DEVINFO_THERMAL_SOC_FLOOR_TEMP	14
+#define QAIC_DEVINFO_THERMAL_SOC_CEILING_TEMP	15
+#define QAIC_DEVINFO_VOLTAGE_CX			16
+#define QAIC_DEVINFO_VOLTAGE_MX			17
+#define QAIC_DEVINFO_VOLTAGE_NSP_CX		18
+#define QAIC_DEVINFO_VOLTAGE_NSP_MX		19
+#define QAIC_DEVINFO_CFG_TOPS			20
+#define QAIC_DEVINFO_THERMAL_PMIC_TEMP_A	21
+#define QAIC_DEVINFO_THERMAL_PMIC_TEMP_C	22
+#define QAIC_DEVINFO_THERMAL_PMIC_TEMP_E	23
+#define QAIC_DEVINFO_POWER_ACTION		24
+#define QAIC_DEVINFO_SOC_TDP			25
+#define QAIC_DEVINFO_SOC_POWER			26
+
 /**
  * struct qaic_manage_trans_hdr - Header for a transaction in a manage message.
  * @type: In. Identifies this transaction. See QAIC_TRANS_* defines.
-- 
2.43.0

Re: [PATCH 3/3] accel/qaic: Add callbacks for DRM RAS netlink telemetry

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] accel/qaic: NULL pointer dereference of qdev->tele_ch during telemetry requests
- [High] accel/qaic: Privilege escalation in qaic_telemetry_pre via mount namespace bypass
- [High] accel/qaic: Missing cleanup in qaic_pci_probe error paths causes UAF and leaks
- [High] qaic: UAF of qdev in tele_wq due to mismatched devm and drmm lifecycles
- [Medium] accel/qaic: Memory leak of drm_ras_node in qaic_drm_ras_register_node error path
- [Medium] accel/qaic: Misaligned DMA buffer embedded in struct wrapper_msg
- [Low] accel/qaic: Incompatible pointer type warning in qaic_telemetry_read/write
--

commit bade5a85b4c68c8d3389f9341f8bbf308ca4eee4
Author: Zack McKevitt <zachary.mckevitt@oss.qualcomm.com>

accel/qaic: Add callbacks for DRM RAS netlink telemetry
    
Use the DRM RAS netlink infrastructure to perform device telemetry.
During PCI probe, create a DRM RAS node with DRM_RAS_NODE_TYPE_TELEMETRY
type and add callbacks to telemetry_request for read and write operations.

> diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
> index c1bb6db78d129..d2aed51052a66 100644
> --- a/drivers/accel/qaic/qaic_drv.c
> +++ b/drivers/accel/qaic/qaic_drv.c
[ ... ]
> @@ -443,6 +448,9 @@ static struct qaic_device *create_qdev(struct pci_dev *pdev,
>  	qdev->ssr_wq = qaicm_wq_init(drm, "qaic_ssr");
>  	if (IS_ERR(qdev->ssr_wq))
>  		return NULL;
> +	qdev->tele_wq = qaicm_wq_init(drm, "qaic_tele");
> +	if (IS_ERR(qdev->tele_wq))
> +		return NULL;

[Severity: High]
Can this mismatched devm and drmm lifecycle lead to a use-after-free?

The qdev structure is allocated via devm_kzalloc() and is tied to the PCI
device lifecycle, getting freed immediately upon unbind. However, tele_wq
is tied to the DRM device lifecycle via drmm_add_action_or_reset().

If userspace holds the DRM file descriptor open during PCI device unbind,
the DRM device will outlive qdev. Any queued resp_worker() items running
on tele_wq will execute after qdev is freed and dereference it.

[ ... ]
> @@ -595,6 +604,10 @@ static int qaic_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (ret)
>  		return ret;
>  
> +	ret = qaic_drm_ras_register_node(qdev);
> +	if (ret)
> +		return ret;
> +

[Severity: High]
If qaic_drm_ras_register_node() fails here, we return an error directly
without calling qaic_destroy_drm_device(). Doesn't this leak the registered
DRM device?

Also, if qaic_mhi_register_controller() fails immediately after this, it
calls qaic_destroy_drm_device() but fails to call
qaic_drm_ras_unregister_node(). Since qdev is managed by devm_kzalloc and
will be freed by the driver core on probe failure, leaving the DRM RAS node
or DRM device registered with dangling pointers to the freed qdev will lead
to Use-After-Free crashes if userspace attempts to interact with them.

[Severity: High]
The telemetry node is exposed to userspace here before the MHI channel
is initialized on the next line. 

In qaic_pci_remove(), the MHI controller is freed (which asynchronously
clears qdev->tele_ch = NULL) before the DRM RAS node is unregistered.

Concurrent netlink requests from userspace during either of these windows
will trigger a NULL pointer dereference in mhi_queue_buf(), causing a
kernel oops, because qdev->tele_ch is accessed unconditionally in
telemetry_request().

>  	qdev->mhi_cntrl = qaic_mhi_register_controller(pdev, qdev->bar_mhi, mhi_irq,
>  						       qdev->single_msi, config->family);
[ ... ]
> diff --git a/drivers/accel/qaic/qaic_telemetry.c b/drivers/accel/qaic/qaic_telemetry.c
> new file mode 100644
> index 0000000000000..dbfb36cf319ce
> --- /dev/null
> +++ b/drivers/accel/qaic/qaic_telemetry.c
[ ... ]
> +int telemetry_request(struct qaic_device *qdev, u8 cmd, u8 cmd_type, s64 *val)
> +{
[ ... ]
> +	kref_get(&wrapper->ref_count);
> +	ret = mhi_queue_buf(qdev->tele_ch, DMA_TO_DEVICE, req, sizeof(*req), MHI_EOT);

[Severity: Medium]
Is it safe to pass this embedded buffer to the DMA engine?

The req buffer is a pointer to wrapper->msg, which is embedded inside
struct wrapper_msg immediately after struct kref ref_count (4 bytes).
This means the req buffer starts at a 4-byte offset from the kzalloc
allocation, sharing its cacheline with the kref.

Passing unaligned buffers to the DMA mapping API violates architecture DMA
requirements and can cause cacheline tearing and memory corruption on
non-cache-coherent architectures.

> +	if (ret) {
[ ... ]
> +static int qaic_telemetry_pre(struct drm_ras_node *ep)
> +{
> +	struct qaic_device *qdev = ep->priv;
> +	struct inode *dev_inode;
> +	struct path path;
> +	char devpath[24];
> +	int ret;
> +
> +	snprintf(devpath, sizeof(devpath), "/dev/accel/%s", to_accel_kdev(qdev->qddev)->kobj.name);
> +	ret = kern_path(devpath, LOOKUP_FOLLOW, &path);
> +	if (ret)
> +		return -EINVAL;
> +
> +	dev_inode = d_inode(path.dentry);
> +	ret = inode_permission(&nop_mnt_idmap, dev_inode, MAY_READ | MAY_WRITE);

[Severity: High]
Does this regression introduce a privilege escalation vector by relying on
the caller's mount namespace for security enforcement?

kern_path() resolves paths relative to the caller's root and mount
namespace. An unprivileged attacker could use unshare to create a new
mount namespace, mount a tmpfs over /dev/accel, and create a fake
device node with 0777 permissions.

When the attacker sends a netlink request, kern_path() would resolve to
their fake file, passing the inode_permission() check and granting
unauthorized execution of privileged telemetry commands.

> +	path_put(&path);
> +
> +	return ret;
> +}
> +
> +static int qaic_telemetry_read(struct drm_ras_node *ep, u32 *field, u64 *value, s32 *status)
> +{
> +	struct qaic_device *qdev = ep->priv;
> +
> +	*status = (s32)telemetry_request(qdev, *field, TYPE_READ, value);

[Severity: Low]
Will this implicit cast between signed and unsigned pointer types trigger
-Wincompatible-pointer-types warnings and break builds with -Werror?

qaic_telemetry_read() receives a u64 *value parameter from the DRM RAS
API, but it passes it directly to telemetry_request(), which takes an
s64 *val parameter.

> +
> +	return *status;
> +}
[ ... ]
> +int qaic_drm_ras_register_node(struct qaic_device *qdev)
> +{
> +	struct drm_ras_node *node;
[ ... ]
> +	ret = drm_ras_node_register(node);
> +	if (ret) {
> +		kfree(device_name);
> +		kfree(node_name);
> +		kfree(qdev->ras_node);

[Severity: Medium]
Should this be kfree(node) instead?

qdev->ras_node is not assigned until the end of the function, so it is
NULL at this point. kfree(NULL) is a no-op, which means the node
allocation is permanently leaked on this error path.

> +		return ret;
> +	}
> +
> +	qdev->ras_node = node;
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009173152.2436195-1-zachary.mckevitt@oss.qualcomm.com?part=3

[syzbot ci] Re: Extend drm_ras for qaic telemetry

[syzbot ci]

syzbot ci has tested the following series

[v1] Extend drm_ras for qaic telemetry
https://lore.kernel.org/all/20261009173152.2436195-1-zachary.mckevitt@oss.qualcomm.com
* [PATCH 1/3] drm/drm_ras: Remove admin-perm flag from list-nodes operation
* [PATCH 2/3] drm/drm_ras: Extend DRM RAS interface to include telemetry nodes
* [PATCH 3/3] accel/qaic: Add callbacks for DRM RAS netlink telemetry

and found the following issue:
general protection fault in drm_ras_nl_pre_telemetry

Full report is available here:
https://ci.syzbot.org/series/6915fb7c-08cd-48b4-b8a4-3834f3e60de5

***

general protection fault in drm_ras_nl_pre_telemetry

tree:      drm-next
URL:       https://gitlab.freedesktop.org/drm/kernel.git
base:      f2bf20011b95401f63e573d54134544c452c4117
arch:      amd64
compiler:  Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
config:    https://ci.syzbot.org/builds/d48f7944-a0fb-4983-a3c6-c037c81dd832/config
syz repro: https://ci.syzbot.org/findings/c2ca90ba-3f79-4e56-94d4-b1ddc64056e5/syz_repro

Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
CPU: 1 UID: 0 PID: 5808 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:nla_get_u32 include/net/netlink.h:1734 [inline]
RIP: 0010:drm_ras_nl_pre_telemetry+0x60/0xe0 drivers/gpu/drm/drm_ras.c:656
Code: 8b 1b 48 83 c3 08 48 89 d8 48 c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 3e 3c 9a fc 48 8b 1b 48 83 c3 04 48 89 d8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 75 5b 8b 33 48 c7 c7 80 d3 6f 90 e8 c9 8c 49
RSP: 0018:ffffc9000379f420 EFLAGS: 00010247
RAX: 0000000000000000 RBX: 0000000000000004 RCX: ffff8881bf17bc00
RDX: 0000000000000000 RSI: ffff8881bf38d380 RDI: ffffc9000379f600
RBP: ffffc9000379f588 R08: ffffc9000379f4ff R09: 0000000000000000
R10: ffffc9000379f4d0 R11: ffffffff859eb380 R12: ffffc9000379f600
R13: dffffc0000000000 R14: ffff8881bf38d380 R15: dffffc0000000000
FS:  00007f2c256e36c0(0000) GS:ffff8882a7cb6000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000200000000100 CR3: 000000016e7ee000 CR4: 00000000000006f0
Call Trace:
 <TASK>
 genl_family_rcv_msg_doit+0x1db/0x340 net/netlink/genetlink.c:1109
 genl_family_rcv_msg net/netlink/genetlink.c:1194 [inline]
 genl_rcv_msg+0x614/0x7a0 net/netlink/genetlink.c:1209
 netlink_rcv_skb+0x226/0x4a0 net/netlink/af_netlink.c:2575
 genl_rcv+0x28/0x40 net/netlink/genetlink.c:1218
 netlink_unicast_kernel net/netlink/af_netlink.c:1338 [inline]
 netlink_unicast+0x7bd/0x940 net/netlink/af_netlink.c:1364
 netlink_sendmsg+0x813/0xb40 net/netlink/af_netlink.c:1919
 sock_sendmsg_nosec+0x13a/0x180 net/socket.c:800
 __sock_sendmsg net/socket.c:815 [inline]
 ____sys_sendmsg+0x54e/0x850 net/socket.c:2713
 ___sys_sendmsg+0x2a5/0x360 net/socket.c:2767
 __sys_sendmsg net/socket.c:2799 [inline]
 __do_sys_sendmsg net/socket.c:2804 [inline]
 __se_sys_sendmsg net/socket.c:2802 [inline]
 __x64_sys_sendmsg+0x1b1/0x290 net/socket.c:2802
 do_syscall_x64 arch/x86/entry/syscall_64.c:61 [inline]
 do_syscall_64+0x166/0x520 arch/x86/entry/syscall_64.c:84
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f2c2479e1d9
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f2c256e3028 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007f2c24a25fa0 RCX: 00007f2c2479e1d9
RDX: 0000000000000000 RSI: 0000200000000100 RDI: 0000000000000004
RBP: 00007f2c24835888 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f2c24a26038 R14: 00007f2c24a25fa0 R15: 00007ffc88247b58
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:nla_get_u32 include/net/netlink.h:1734 [inline]
RIP: 0010:drm_ras_nl_pre_telemetry+0x60/0xe0 drivers/gpu/drm/drm_ras.c:656
Code: 8b 1b 48 83 c3 08 48 89 d8 48 c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 3e 3c 9a fc 48 8b 1b 48 83 c3 04 48 89 d8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 75 5b 8b 33 48 c7 c7 80 d3 6f 90 e8 c9 8c 49
RSP: 0018:ffffc9000379f420 EFLAGS: 00010247
RAX: 0000000000000000 RBX: 0000000000000004 RCX: ffff8881bf17bc00
RDX: 0000000000000000 RSI: ffff8881bf38d380 RDI: ffffc9000379f600
RBP: ffffc9000379f588 R08: ffffc9000379f4ff R09: 0000000000000000
R10: ffffc9000379f4d0 R11: ffffffff859eb380 R12: ffffc9000379f600
R13: dffffc0000000000 R14: ffff8881bf38d380 R15: dffffc0000000000
FS:  00007f2c256e36c0(0000) GS:ffff8882a7cb6000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f2c247eb8c0 CR3: 000000016e7ee000 CR4: 00000000000006f0
----------------
Code disassembly (best guess):
   0:	8b 1b                	mov    (%rbx),%ebx
   2:	48 83 c3 08          	add    $0x8,%rbx
   6:	48 89 d8             	mov    %rbx,%rax
   9:	48 c1 e8 03          	shr    $0x3,%rax
   d:	42 80 3c 38 00       	cmpb   $0x0,(%rax,%r15,1)
  12:	74 08                	je     0x1c
  14:	48 89 df             	mov    %rbx,%rdi
  17:	e8 3e 3c 9a fc       	call   0xfc9a3c5a
  1c:	48 8b 1b             	mov    (%rbx),%rbx
  1f:	48 83 c3 04          	add    $0x4,%rbx
  23:	48 89 d8             	mov    %rbx,%rax
  26:	48 c1 e8 03          	shr    $0x3,%rax
* 2a:	42 0f b6 04 38       	movzbl (%rax,%r15,1),%eax <-- trapping instruction
  2f:	84 c0                	test   %al,%al
  31:	75 5b                	jne    0x8e
  33:	8b 33                	mov    (%rbx),%esi
  35:	48 c7 c7 80 d3 6f 90 	mov    $0xffffffff906fd380,%rdi
  3c:	e8                   	.byte 0xe8
  3d:	c9                   	leave
  3e:	8c                   	.byte 0x8c
  3f:	49                   	rex.WB


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

To test a fix for this bug, please reply with `#syz test`
(on a separate line) and attach the patch to the email.

Notes:
- The patch will be applied on top of the tested series (as an
  incremental fix).
- To test a new version of the whole series, please send it directly
  to syzbot@lists.linux.dev.
- Arguments like custom git repos and branches are not supported.