Re: [PATCH v5 07/14] gtrace: Add function to copy into perf AUX buffer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The newly added `gtrace_path_copyto_auxbuf` function dereferences `gtdrv` without a NULL check, leading to a kernel panic if the component's driver is unbound.
- [High] Lack of module pinning or device locking in `gtrace_path_copyto_auxbuf` allows concurrent driver unbind or module unloading, leading to a Use-After-Free and kernel panic.
--

--- Patch [7]: [PATCH v5 07/14] gtrace: Add function to copy into perf AUX buffer ---
commit 175afd4c59c793de1c5fef3899d7379cf730bece
Author: Anup Patel <anup.patel@oss.qualcomm.com>

gtrace: Add function to copy into perf AUX buffer

This commit adds gtrace_path_copyto_auxbuf() and a new driver callback
copyto_auxbuf to allow the RISC-V trace ramsink to copy trace data into a
perf AUX buffer.

> diff --git a/drivers/hwtracing/gtrace/gtrace-core.c b/drivers/hwtracing/gtrace/gtrace-core.c
> index 6ff1b30879472..53a3679ba69fa 100644
> --- a/drivers/hwtracing/gtrace/gtrace-core.c
> +++ b/drivers/hwtracing/gtrace/gtrace-core.c
> @@ -626,6 +626,28 @@ int gtrace_path_stop(struct gtrace_path *path)
>  }
>  EXPORT_SYMBOL_GPL(gtrace_path_stop);
>  
> +int gtrace_path_copyto_auxbuf(struct gtrace_path *path,
> +			      struct gtrace_perf_auxbuf *buf,
> +			      size_t *bytes_copied, u64 *format)
> +{
> +	const struct gtrace_driver *gtdrv;
> +	struct gtrace_component *comp;
> +	struct gtrace_path_node *node;
> +
> +	list_for_each_entry(node, &path->comp_list, head) {
> +		comp = node->comp;
> +		gtdrv = to_gtrace_driver(comp->dev.driver);
> +		if (!gtdrv->copyto_auxbuf)

[Severity: High]
Does this sequence risk a null pointer dereference if a component's driver is
unbound?

If a driver is dynamically unbound via sysfs while the perf session is active,
comp->dev.driver becomes null. The to_gtrace_driver() macro will then return
null, causing the evaluation of gtdrv->copyto_auxbuf to panic.

> +			continue;
> +
> +		*bytes_copied = gtdrv->copyto_auxbuf(comp, buf, format);

[Severity: High]
Can this indirect function call race with a concurrent module unload?

If the component's module is unloaded immediately after the driver pointer is
checked but before the callback executes, jumping to gtdrv->copyto_auxbuf()
could execute freed memory.

Could this be prevented by holding a module reference (try_module_get()) or
taking the device lock (device_lock()) while reading the driver structure and
executing the callback?

> +		return 0;
> +	}
> +
> +	return -EOPNOTSUPP;
> +}

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