Re: [PATCH v3 11/22] accel: ethosu: Validate secondary streams

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Bypass of bounds validation for secondary weight and scale streams due to 'optional' logic, allowing use of stale register values.

Pre-existing issues:
- [Critical] Missing bounds validation for the scale stream in elementwise operations.
--

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

accel: ethosu: Validate secondary streams

The command-stream validator records the additional U65 scale and weight
stream addresses and the U85 weight decoder addresses, but only checked
stream 0 against its region buffer. This commit checks every configured
secondary stream against the matching weight or scale region before
accepting a kernel operation.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index 6f6712f4efd1f..abfb173e10080 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -436,13 +436,17 @@ static u64 feat_matrix_length(struct ethosu_device *edev,
>  
>  static int buffer_size(struct ethosu_validated_cmdstream_info *info,
>  		       struct cmd_state *st, struct buffer *buf, s8 region,
> -		       u16 region_cmd, u16 base_cmd, u16 length_cmd)
> +		       u16 region_cmd, u16 base_cmd, u16 length_cmd, bool optional)
>  {
>  	u64 end;
> +	bool base_set = cmd_state_reg_is_set(st, base_cmd);
> +	bool length_set = cmd_state_reg_is_set(st, length_cmd);
> +
> +	if (optional && !base_set && !length_set)
> +		return 0;

[Severity: Critical]
Does this logic allow omitted streams to bypass bounds validation while
the hardware might still use them?

If a userspace application omits the base and length commands for secondary
streams that are inherently required by an operation (such as a multi-core
convolution), this check returns 0 and skips bounds validation.

Since Ethos-U NPUs retain register state between jobs, the hardware might
execute the operation using stale offsets from a previous job. Could this
lead to an out-of-bounds memory access by the NPU if the current job's
regions are smaller than those of the previous job?

[ ... ]

[Severity: Critical]
This is a pre-existing issue, but does calc_sizes_elemwise() miss bounds
validation for the scale stream?

In ethosu_gem_cmdstream_copy_and_validate(), a use_scale variable is
computed but completely ignored when calling calc_sizes_elemwise():

drivers/accel/ethosu/ethosu_gem.c:ethosu_gem_cmdstream_copy_and_validate() {
    ...
    ret = calc_sizes_elemwise(ddev, info, cmd, &st, use_ifm, use_ifm2);
    ...
}

And calc_sizes_elemwise() entirely lacks logic to validate the st.scale
buffers.

If an unprivileged userspace application provides arbitrary, out-of-bounds
NPU_SET_SCALE_BASE and LENGTH values in the command stream for an
NPU_OP_ELEMENTWISE operation, the hardware will read from the unvalidated
offset. Can this allow an attacker to access arbitrary NPU-mapped memory?

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