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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unvalidated device tree properties allow out-of-bounds array access and memory corruption.
- [High] Unconditional hardware state reset clobbers interrupt states and wait bases for unowned syncpoints.
--

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

gpu: host1x: Allow limiting usable channel and syncpoint ranges

Allow 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 and Linux has limited access to them.

> 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);

[Severity: High]
Does this code allow out-of-bounds memory accesses if the device tree values
exceed the hardware limits?

Since chlist->channels and chlist->allocated_channels are allocated based on
host->info->nb_channels, could a large value in the nvidia,channels device
tree property cause an out-of-bounds access here when host->channel_end is
passed to find_next_zero_bit()? Should host->channel_end be validated
in host1x_get_assigned_resources()?

[ ... ]

> 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];
> +	} 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, could large values for nvidia,syncpoints cause out-of-bounds reads
and writes?

Arrays like host->syncpt are allocated based on host->info->nb_pts.
If host->syncpt_end exceeds this limit without validation, functions like
host1x_syncpt_alloc() that iterate up to host->syncpt_end might access
memory out of bounds.

[ ... ]

> diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c
> index acc7d82e0585e..fe27a386cc0c8 100644
> --- a/drivers/gpu/host1x/syncpt.c
> +++ b/drivers/gpu/host1x/syncpt.c
[ ... ]
> @@ -138,7 +138,7 @@ void host1x_syncpt_restore(struct host1x *host)
>  	struct host1x_syncpt *sp_base = host->syncpt;
>  	unsigned int i;
>  
> -	for (i = 0; i < host1x_syncpt_nb_pts(host); i++) {
> +	for (i = host->syncpt_base; i < host->syncpt_end; i++) {
>  		/*
>  		 * Unassign syncpt from channels for purposes of Tegra186
>  		 * syncpoint protection. This prevents any channel from

[Severity: High]
Does this clobber the hardware state for unowned syncpoints?

Right after this loop, the code in host1x_syncpt_restore() unconditionally
restores wait bases for syncpoints 0 to nb_bases-1:

	for (i = 0; i < host1x_syncpt_nb_bases(host); i++)
		host1x_hw_syncpt_restore_wait_base(host, sp_base + i);

Similarly, host1x_intr_disable_all_syncpt_intrs() unconditionally disables and
clears threshold interrupts for all syncpoints across the entire hardware.

Since this patch restricts Linux to a specific range of syncpoints to allow
sharing the Host1x device, will overwriting the wait bases and interrupt states
unconditionally break synchronization for other operating systems or hardware
units sharing the device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-host1x-nohv-v1-0-65bc682a617f@nvidia.com?part=4