Re: [PATCH v3 14/22] accel: ethosu: Account for feature map element size

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Missing max coordinate check for tile 0 in feat_matrix_size when storage == 1 allows out-of-bounds NPU memory accesses.
--

commit 8f5e0f4e084740b6432179e0d2b5e40f78355e1e
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Account for feature map element size

The feature map bounds calculation currently accounts for only one byte
of the final element. This can leave the tail of B16, B32, and B64
elements outside the validated buffer.

Use the feature map element size when calculating the final byte.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index 11aa3f4dd0e7d..974cf562ef495 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -426,7 +426,7 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
>  		return U64_MAX;
>  	}
>  
> -	if (check_add_overflow(addr, 1ULL, &offset))
> +	if (check_add_overflow(addr, (u64)element_size, &offset))
>  		return U64_MAX;
>  
>  	info->region_size[fm->region] = max(info->region_size[fm->region], offset);

[Severity: Critical]
This is a pre-existing issue, but while we are looking at feature map
bounds calculations, I noticed a potential out-of-bounds regression in
feat_matrix_size() introduced earlier in the series.

When storage == 1 (plan 1 vertical tiling) and the operation spans into
tile 1 or tile 2 (where y > fm->height[0]), the validation only checks y=0
for tile 0:

drivers/accel/ethosu/ethosu_gem.c:feat_matrix_size() {
...
	if (storage == 1) {
		ret = feat_matrix_check_location(edev, info, st, fm, type, x, 0, c,
						 ofm, max_len);
...
	}
...

Since base[0] and base[1] are independently programmed by the user-space
command stream, base[0] + fm->height[0] * stride_y can be the true global
maximum address of the feature map.

Because the check omits the maximum boundary of tile 0 (which would be
min(y, fm->height[0])), could this severely underestimate the required
buffer size and allow unprivileged NPU command streams to access memory
outside the validated GEM buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-ethosu-fixes-v3-0-490fe215286f@kernel.org?part=14