Re: [PATCH v19 3/7] coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The error handling path in `__tmc_probe()` at the `out:` label unconditionally accesses `drvdata->csdev` and attempts to register the crash metadata interface, leading to NULL pointer dereference, error pointer dereference, or Use-After-Free.
--

commit 0e2c5d1f36c04454695412377951547f5c57c845
Author: Jie Gan <jie.gan@oss.qualcomm.com>

coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations

Introduce tmc_sysfs_ops as a wrapper, wrap sysfs read operations,
for reading trace data from the TMC buffer.

> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index bc7dd676da473..4b40b692be4d8 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c

[ ... ]

> @@ -835,6 +826,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
>  		idr_init(&drvdata->idr);
>  		mutex_init(&drvdata->idr_mutex);
>  		dev_list = "tmc_etr";
> +		drvdata->sysfs_ops = &etr_sysfs_ops;
>  		INIT_LIST_HEAD(&drvdata->etr_buf_list);
>  		break;
>  	case TMC_CONFIG_TYPE_ETF:
> @@ -844,6 +836,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
>  		desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO;
>  		desc.ops = &tmc_etf_cs_ops;
>  		dev_list = "tmc_etf";
> +		drvdata->sysfs_ops = &etb_sysfs_ops;
>  		break;
>  	default:
>  		pr_err("%s: Unsupported TMC config\n", desc.name);

[Severity: High]
This isn't a bug introduced by this patch, but looking at __tmc_probe(),
is it safe to unconditionally call the crashdata functions at the out: label
if probe fails?

If an error occurs during probe (for example, coresight_alloc_device_name()
returns NULL, or coresight_register() returns an ERR_PTR), the code jumps
to out:

out:
	if (is_tmc_crashdata_valid(drvdata) &&
	    !tmc_prepare_crashdata(drvdata))
		register_crash_dev_interface(drvdata, desc.name);

is_tmc_crashdata_valid() then attempts to access drvdata->csdev->dev:

drivers/hwtracing/coresight/coresight-tmc-core.c:is_tmc_crashdata_valid() {
    ...
	/* Check for valid metadata */
	if (!mdata->valid) {
		dev_dbg(&drvdata->csdev->dev,
			"Data invalid in tmc crash metadata\n");
    ...
}

Will this cause a NULL pointer or ERR_PTR dereference if drvdata->csdev
hasn't been successfully registered?

Furthermore, if misc_register() fails, coresight_unregister() frees csdev,
potentially leading to a use-after-free. If register_crash_dev_interface()
succeeds on an error path, could the driver core subsequently free drvdata
via devres cleanup, leaving a dangling pointer in the global miscdevice list?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com?part=3