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) <[email protected]> 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/[email protected]?part=11
