Re: [PATCH v15 3/7] qcom-tgu: Add signal priority support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Sysfs files are exposed to userspace before their backing memory is allocated, creating a race condition that leads to a NULL pointer dereference.
- [High] Missing cleanup of explicitly created sysfs groups on probe error paths leads to dangling sysfs nodes and potential Use-After-Free.
- [Low] Missing spinlock acquisition or `READ_ONCE` in `tgu_dataset_show` causes a data race with concurrent sysfs writes.
--

commit bffbcf9cda751c587a916ac6240800447250831c
Author: Songwei Chai <songwei.chai@oss.qualcomm.com>

qcom-tgu: Add signal priority support
    
Like circuit of a Logic analyzer, in TGU, the requirement could be
configured in each step and the trigger will be created once the
requirements are met. Add priority functionality here to sort the
signals into different priorities. The signal which is wanted could
be configured in each step's priority node, the larger number means
the higher priority and the signal with higher priority will be sensed
more preferentially.

> diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c
> index 49c8f710b9318..7d69986c3e3d2 100644
> --- a/drivers/hwtracing/qcom/tgu.c
> +++ b/drivers/hwtracing/qcom/tgu.c
> @@ -14,14 +14,123 @@
>  
>  #include "tgu.h"
>  

[ ... ]

> +static ssize_t tgu_dataset_show(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
> +	struct tgu_attribute *tgu_attr =
> +			container_of(attr, struct tgu_attribute, attr);
> +	int index;
> +
> +	index = calculate_array_location(drvdata, tgu_attr->step_index,
> +					 tgu_attr->operation_index,
> +					 tgu_attr->reg_num);
> +
> +	return sysfs_emit(buf, "0x%x\n",
> +			  drvdata->value_table->priority[index]);

[Severity: Low]
Does this read need to be protected by drvdata->lock or use READ_ONCE()?

tgu_dataset_store() acquires drvdata->lock before writing to this array,
but this read accesses it locklessly, which could cause a data race.

> +}

[ ... ]

> @@ -128,6 +269,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>  {
>  	struct device *dev = &adev->dev;
>  	struct tgu_drvdata *drvdata;
> +	unsigned int *priority;
> +	size_t priority_size;
>  	int ret;
>  
>  	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> @@ -143,12 +286,30 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>  
>  	spin_lock_init(&drvdata->lock);
>  
> +	tgu_set_reg_number(drvdata);
> +	tgu_set_steps(drvdata);
> +
>  	ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
>  	if (ret) {
>  		dev_err(dev, "failed to create sysfs groups: %d\n", ret);
>  		return ret;
>  	}
>  
> +	drvdata->value_table =
> +		devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL);

[Severity: High]
Does this expose sysfs files to userspace before their backing memory is
allocated?

If userspace reads a sysfs file immediately after sysfs_create_groups() returns,
but before the memory allocations below complete, tgu_dataset_show() will
dereference the uninitialized drvdata->value_table pointer. Should these
allocations happen before the sysfs groups are created?

> +	if (!drvdata->value_table)
> +		return -ENOMEM;

[Severity: High]
Does this error path leak the sysfs groups created earlier?

If this allocation fails, the function returns -ENOMEM without calling
sysfs_remove_groups(). The devres cleanup will eventually clear the drvdata
pointer, leaving dangling sysfs nodes. Any subsequent access to these files
would then trigger a NULL pointer dereference or use-after-free.

> +
> +	priority_size = MAX_PRIORITY * drvdata->num_reg * drvdata->num_step;
> +
> +	priority = devm_kcalloc(dev, priority_size,
> +				sizeof(*drvdata->value_table->priority),
> +				GFP_KERNEL);
> +	if (!priority)
> +		return -ENOMEM;
> +
> +	drvdata->value_table->priority = priority;
>  
>  	drvdata->enabled = false;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701055529.445819-1-songwei.chai@oss.qualcomm.com?part=3