Re: [PATCH v6 03/14] gtrace: Add RISC-V platform driver for the gtrace framework

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] of: use-after-free of device node in rvtrace_platform_probe()
- [High] core: use-after-free hazard in rvtrace_platform_remove() during unregistration
- [Medium] of: memory leak of rep_node and rdev_node in gtrace_of_parse_outconns()
- [Medium] of: fwnode handle leak on -EPROBE_DEFER error path in gtrace_of_parse_outconns()
- [Medium] of: fwnode handle references leaked on rvtrace_platform_probe() error paths
- [Medium] module: missing MODULE_DEVICE_TABLE prevents automatic driver loading
--

commit 70aceab46d8019c787fc4aee6cb3f8a0b2772885
Author: Anup Patel <anup.patel@oss.qualcomm.com>

gtrace: Add RISC-V platform driver for the gtrace framework

The RISC-V trace components defined by the RISC-V Trace Control Interface
Specification are memory mapped devices which are organized in a
graph-like topology and described in the device tree.

Add the RISC-V platform driver for the gtrace framework. The driver
parses the RISC-V component topology from the device tree, discovers the
type and version of each component from its IMPL register, provides the
RISC-V register level control ops and registers each component with the
gtrace core.

> diff --git a/drivers/hwtracing/gtrace/gtrace-of.c b/drivers/hwtracing/gtrace/gtrace-of.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/hwtracing/gtrace/gtrace-of.c
[ ... ]
> +		conn->dest_port = rep.port;
> +		conn->dest_fwnode = of_fwnode_handle(rdev_node);
> +		fwnode_handle_get(conn->dest_fwnode);
> +		conn->dest_comp = gtrace_find_by_fwnode(conn->dest_fwnode);
> +		if (!conn->dest_comp) {
> +			ret = -EPROBE_DEFER;
> +			of_node_put(ep_node);
> +			of_node_put(rep_node);
> +			of_node_put(rdev_node);
> +			break;

[Severity: Medium]
Does this error path leak the conn->dest_fwnode reference?

The fwnode reference count was incremented by fwnode_handle_get(), but the
newly allocated conn has not yet been placed into the pdata->outconns array.
Consequently, the error cleanup loop at the done label skips this connection.

> +		}
> +
> +		pdata->outconns[i] = conn;
> +		i++;

[Severity: Medium]
Does the success path of this endpoint parsing loop leak the device node
references?

The of_graph_get_remote_endpoint() and of_graph_get_port_parent() calls
acquire references to device nodes. On the success path, these references
are never released with of_node_put() before advancing to the next iteration.

> +	}
> +
> +done:
> +	if (ret) {
> +		for (i = 0; i < pdata->nr_outconns && pdata->outconns; i++) {
> +			conn = pdata->outconns[i];
> +			if (conn && conn->dest_fwnode)
> +				fwnode_handle_put(conn->dest_fwnode);
> +		}
> +	}
> +	of_node_put(parent);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(gtrace_of_parse_outconns);
[ ... ]
> diff --git a/drivers/hwtracing/gtrace/rvtrace-platform.c b/drivers/hwtracing/gtrace/rvtrace-platform.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/hwtracing/gtrace/rvtrace-platform.c
[ ... ]
> +	pdata->bound_cpu = -1;
> +	node = of_parse_phandle(dev_of_node(dev), "cpus", 0);
> +	if (node) {
> +		ret = of_cpu_node_to_id(node);
> +		of_node_put(node);
> +		if (ret < 0)
> +			return dev_err_probe(dev, ret, "failed to get CPU id for %pOF\n", node);

[Severity: High]
Does this code trigger a use-after-free on the device node?

The node reference is explicitly released via of_node_put() before it is
passed to dev_err_probe(), where the %pOF specifier will dereference the
now-freed node pointer to extract its name.

> +		pdata->bound_cpu = ret;
> +	}
> +
> +	/* Default control poll timeout */
> +	pdata->control_poll_timeout_usecs = 10;
> +
> +	ret = gtrace_of_parse_outconns(pdata);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to parse output connections\n");
> +
> +	ret = gtrace_of_parse_inconns(pdata);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to parse input connections\n");
> +
> +	/* Reset the component before it is registered with the gtrace core. */
> +	ret = rvtrace_hw_reset(pdata);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to reset component\n");

[Severity: Medium]
Do these error paths leak the fwnode references stored in the outconns array?

The earlier call to gtrace_of_parse_outconns() allocates and acquires fwnode
references, storing them in pdata->outconns. If a subsequent call fails, the
probe function immediately returns an error. Because devm does not
automatically release fwnode references, and the custom teardown logic in
rvtrace_platform_remove() is bypassed on probe failure, these references
appear to be lost.

> +
> +	impl = gtrace_read32(pdata, RVTRACE_COMPONENT_IMPL_OFFSET);
[ ... ]
> +static void rvtrace_platform_remove(struct platform_device *pdev)
> +{
> +	struct gtrace_component *comp = platform_get_drvdata(pdev);
> +	struct gtrace_platform_data *pdata = comp->pdata;
> +	struct gtrace_connection *conn;
> +	int i;
> +
> +	for (i = 0; i < pdata->nr_outconns; i++) {
> +		conn = pdata->outconns[i];
> +		if (conn && conn->dest_fwnode)
> +			fwnode_handle_put(conn->dest_fwnode);
> +	}
> +
> +	gtrace_unregister_component(comp);

[Severity: High]
Does this sequence introduce a use-after-free hazard during unregistration?

The fwnode references are prematurely dropped before unregistering the
component. However, gtrace_unregister_component() relies on these pointers
internally during its teardown process, matching the fwnode device elements.
If the prior fwnode_handle_put() was the final reference, the memory is
freed, and the subsequent match access is a use-after-free.

> +}
> +
> +static const struct of_device_id rvtrace_platform_match[] = {
> +	{ .compatible = "riscv,trace-component", .data = (void *)GTRACE_FORMAT_ETRACE },
> +	{}
> +};

[Severity: Medium]
Is MODULE_DEVICE_TABLE(of, rvtrace_platform_match) missing here?

Without it, the driver fails to export the necessary modaliases to userspace,
preventing the driver from loading automatically when its hardware matches
are present in modular builds.

> +
> +static struct platform_driver rvtrace_platform_driver = {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001052243.3042627-1-mayuresh.chitale@oss.qualcomm.com?part=3