Re: [PATCH v2 4/4] gpu: host1x: Allow limiting usable channel and syncpoint ranges

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unvalidated device tree properties `nvidia,channels` and `nvidia,syncpoints` lead to out-of-bounds array accesses.
--

commit c583bec6dab6b936ed8b99cb1df24a3d2f9ac317
Author: Mikko Perttunen <mperttunen@nvidia.com>

gpu: host1x: Allow limiting usable channel and syncpoint ranges

This commit allows specifying usable channel and allocatable syncpoint
ranges through the device tree. This is necessary when Host1x resources
are shared between multiple operating systems or independent hardware units.

> diff --git a/drivers/gpu/host1x/channel.c b/drivers/gpu/host1x/channel.c
> index ca5d0f51cf7da..38fcdda8388ad 100644
> --- a/drivers/gpu/host1x/channel.c
> +++ b/drivers/gpu/host1x/channel.c
> @@ -121,13 +121,13 @@ EXPORT_SYMBOL(host1x_channel_put);
>  static struct host1x_channel *acquire_unused_channel(struct host1x *host)
>  {
>  	struct host1x_channel_list *chlist = &host->channel_list;
> -	unsigned int max_channels = host->info->nb_channels;
>  	unsigned int index;
>  
>  	mutex_lock(&chlist->lock);
>  
> -	index = find_first_zero_bit(chlist->allocated_channels, max_channels);
> -	if (index >= max_channels) {
> +	index = find_next_zero_bit(chlist->allocated_channels,
> +				   host->channel_end, host->channel_base);
> +	if (index >= host->channel_end) {

[Severity: High]
Could this result in an out-of-bounds access if the device tree specifies a
range larger than host->info->nb_channels?

The bitmap chlist->allocated_channels is sized according to the hardware
maximum, but host->channel_end is parsed directly from the device tree
without bounds checking.

[ ... ]

> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
> index 545fa2e3f180c..7103f018cb1bf 100644
> --- a/drivers/gpu/host1x/dev.c
> +++ b/drivers/gpu/host1x/dev.c
> @@ -331,6 +331,39 @@ static void host1x_setup_virtualization_tables(struct host1x *host)
>  	}
>  }
>  
> +static int host1x_get_assigned_resources(struct host1x *host)
> +{
> +	struct device_node *np = host->dev->of_node;
> +	u32 vals[2];
> +	int err;
> +
> +	err = of_property_read_u32_array(np, "nvidia,channels", vals, 2);
> +	if (err == 0) {
> +		host->channel_base = vals[0];
> +		host->channel_end = vals[0] + vals[1];

[Severity: High]
Should we validate these parsed bounds against host->info->nb_channels?

Without a check, a malformed or dynamically loaded device tree with
excessively large ranges can cause out-of-bounds accesses during allocation.

> +	} else if (err == -EINVAL) {
> +		host->channel_base = 0;
> +		host->channel_end = host->info->nb_channels;
> +	} else {
> +		dev_err(host->dev, "invalid nvidia,channels property: %d\n", err);
> +		return err;
> +	}
> +
> +	err = of_property_read_u32_array(np, "nvidia,syncpoints", vals, 2);
> +	if (err == 0) {
> +		host->syncpt_base = vals[0];
> +		host->syncpt_end = vals[0] + vals[1];

[Severity: High]
Similarly, should we check host->syncpt_end against host->info->nb_pts
here to prevent out-of-bounds pointer dereferences later in
host1x_syncpt_alloc()?

[ ... ]

> diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c
> index 807c74fc6a0ac..7760252b3394e 100644
> --- a/drivers/gpu/host1x/syncpt.c
> +++ b/drivers/gpu/host1x/syncpt.c
> @@ -59,7 +59,7 @@ struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host,
>  					  unsigned long flags,
>  					  const char *name)
>  {
> -	struct host1x_syncpt *sp = host->syncpt;
> +	struct host1x_syncpt *sp = host->syncpt + host->syncpt_base;
>  	char *full_name;
>  	unsigned int i;
>  
> @@ -68,10 +68,10 @@ struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host,
>  
>  	mutex_lock(&host->syncpt_mutex);
>  
> -	for (i = 0; i < host->info->nb_pts && kref_read(&sp->ref); i++, sp++)
> +	for (i = host->syncpt_base; i < host->syncpt_end && kref_read(&sp->ref); i++, sp++)

[Severity: High]
Can this loop dereference sp->ref out of bounds if the device tree specifies
a syncpt_end larger than the size of the host->syncpt array?

The underlying array is allocated based on host->info->nb_pts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-host1x-nohv-v2-0-e6d88bac0af6@nvidia.com?part=4