Re: [PATCH v2 13/19] accel: ethosu: Account for kernel dilation in IFM size

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The command stream validation in `feat_matrix_size` fails to check the maximum accessed address within intermediate tiles, allowing an attacker to bypass bounds checks and cause the NPU to access out-of-bounds memory.
--

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

accel: ethosu: Account for kernel dilation in IFM size

Kernel dilation increases the input feature-map area accessed by a kernel
operation. Include the x and y dilation settings when calculating the
required IFM bounds. Without this, a command stream using x2 dilation can
access IFM memory past the range validated by the driver.

> diff --git a/drivers/accel/ethosu/ethosu_device.h b/drivers/accel/ethosu/ethosu_device.h
> index d7e1e3c8ca12d..8e23fdbf7f8ab 100644
> --- a/drivers/accel/ethosu/ethosu_device.h
> +++ b/drivers/accel/ethosu/ethosu_device.h
[ ... ]
> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index d60a2a453302b..63dd07791f314 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -536,10 +536,16 @@ static int calc_sizes(struct drm_device *ddev,
>  			((st->ifm.stride_kernel >> 1) & 0x1) + 1;
>  		u32 stride_x = ((st->ifm.stride_kernel >> 5) & 0x2) +
>  			(st->ifm.stride_kernel & 0x1) + 1;
> +		u32 dilation_y = 1 + !!(st->ifm.stride_kernel &
> +					 NPU_KERNEL_DILATION_Y);
> +		u32 dilation_x = 1 + !!(st->ifm.stride_kernel &
> +					 NPU_KERNEL_DILATION_X);
>  		s32 ifm_height = st->ofm.height[2] * stride_y +
> -			st->ifm.height[2] - (st->ifm.pad_top + st->ifm.pad_bottom);
> +			st->ifm.height[2] * dilation_y -
> +			(st->ifm.pad_top + st->ifm.pad_bottom);
>  		s32 ifm_width = st->ofm.width * stride_x +
> -			st->ifm.width - (st->ifm.pad_left + st->ifm.pad_right);
> +			st->ifm.width * dilation_x -
> +			(st->ifm.pad_left + st->ifm.pad_right);
>  
>  		if (ifm_height < 0 || ifm_width < 0)
>  			return -EINVAL;

[Severity: Critical]
This isn't a regression introduced by this patch, but does the command stream
validation in feat_matrix_size() fail to check the maximum accessed address
within intermediate tiles?

Looking at drivers/accel/ethosu/ethosu_gem.c in feat_matrix_size():

	if (ethosu_is_u65(edev) || storage == 0) {
		for (int xi = 0; xi < 2; xi++) {
			for (int yi = 0; yi < 2; yi++) {
				ret = feat_matrix_check_location(edev, info, st, fm, type,
								 xi ? x : 0,
								 yi ? y : 0, c, ofm,
								 max_len);

The validation loop only checks the memory bounds by testing the origin
of the first tile (xi = 0, yi = 0) and the extremities of the last tile
(xi = 1, yi = 1), but skips checking the inner boundaries of
intermediate tiles.

If a user-space application constructs a command stream
defining a feature map that spans multiple tiles (e.g., storage == 0),
could it set base[0] near the end of the valid GEM buffer and set a
large width0 for the first tile, while setting base[1] to the beginning
of the buffer?

Because the check for x = 0 (Tile 0) uses base[0] + 0, and the check
for x = x_max (Tile 1) uses base[1] + offset, both could pass
validation. However, the maximum offset within Tile 0
(base[0] + width0 * stride_x) is never verified by the logic.

Could this allow the NPU to execute the command stream and access
out-of-bounds memory up to this unchecked boundary?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=13