Re: [PATCH v3 13/22] accel: ethosu: Validate all feature map tiles

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] In `feat_matrix_size()`, the bounds validation for `storage == 1` (NHCWB16) fails to check the maximum `y` coordinate accessed within Tile 0, allowing arbitrary out-of-bounds NPU memory access.
--

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

accel: ethosu: Validate all feature map tiles

The command-stream validator previously checked only the final feature-map
coordinate, which could leave earlier tile base addresses unchecked for
tiled tensors. This patch checks the final coordinate of every tile touched
by an operation and treats U65 feature maps as 2x2 tiled.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index 5d4e897831396..11aa3f4dd0e7d 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c

[ ... ]

> @@ -434,6 +434,94 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
> +static int feat_matrix_size(struct ethosu_device *edev,
> +			    struct ethosu_validated_cmdstream_info *info,
> +			    struct cmd_state *st, struct feat_matrix *fm,
> +			    enum feat_matrix_type type,
> +			    u32 x, u32 y, u32 c, bool ofm, u64 *max_len)
> +{

[ ... ]

> +	if (storage == 1) {
> +		ret = feat_matrix_check_location(edev, info, st, fm, type, x, 0, c,
> +						 ofm, max_len);
> +		if (ret)
> +			return ret;
> +		if (fm->height[0] < fm->height[1] && fm->height[1] <= y) {

[Severity: High]
This isn't a bug introduced by this patch, but this patch attempts to fix
tile validation and appears to still miss the maximum coordinate of Tile 0
for storage == 1 (NHCWB16).

Does this code fail to validate the upper memory bounds of Tile 0?

When userspace submits a command stream with storage == 1, fm->height[0] > 0,
and an operation maximum y > fm->height[0], the bounds validation evaluates
Tile 0 only at its base (y = 0). It never evaluates min(y, fm->height[0]).

Because the validator calculates the required size for Tile 0 based only on
its first row, it misses the memory offset for subsequent rows up to
fm->height[0]. By supplying an undersized GEM buffer for Tile 0, could an
unprivileged user bypass memory validation and corrupt NPU-accessible memory?

> +			ret = feat_matrix_check_location(edev, info, st, fm, type, x,
> +							 fm->height[1], c, ofm,
> +							 max_len);
> +			if (ret)
> +				return ret;
> +		}
> +		if (fm->height[1] < y) {
> +			ret = feat_matrix_check_location(edev, info, st, fm, type, x,
> +							 fm->height[1] + 1, c, ofm,
> +							 max_len);
> +			if (ret)
> +				return ret;
> +		}
> +	}

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