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

Pre-existing issues:
- [High] The command validator fails to validate the `scale` buffer bounds for 
`NPU_OP_ELEMENTWISE` operations, potentially allowing out-of-bounds NPU memory 
reads.
--

commit ce7a8243218e8f4759fe093516845ea9fe6bb544
Author: Rob Herring (Arm) <[email protected]>

accel: ethosu: Validate convolution parameter

The commit fixes a validation issue with the U65 NPU_OP_CONV command where a
crafted stream could skip the weight buffer validation. It requires a zero
parameter on U65 and rejects reserved U85 parameter bits.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c 
> b/drivers/accel/ethosu/ethosu_gem.c
> index bca68fab05275..d60a2a453302b 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
> @@ -733,7 +733,10 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct 
> drm_device *ddev,
>                               st.dma.dst.region, st.dma.dst.offset, dstlen);
>                       break;
>               case NPU_OP_CONV:
> -                     use_ifm2 = param & 0x1;  // weights_ifm2
> +                     if ((ethosu_is_u65(edev) && param) || (param & 
> ~NPU_OP_CONV_WEIGHTS_IFM2))
> +                             return -EINVAL;
> +
> +                     use_ifm2 = param & NPU_OP_CONV_WEIGHTS_IFM2;
>                       if (!cmd_state_reg_is_set(&st, NPU_SET_OFM_PRECISION))
>                               return -EINVAL;
>                       use_scale = !(st.ofm.precision & 0x100);

[ ... ]

>               case NPU_OP_ELEMENTWISE:
>                       if (!ethosu_is_u65(edev) &&
>                           !cmd_state_reg_is_set(&st, NPU_SET_IFM_BROADCAST))
>                               return -EINVAL;
>                       use_ifm2 = (param != 5) && (param != 6) &&
>                               (param != 7) && (param != 0x24);
>                       if (use_ifm2 &&
>                           !cmd_state_reg_is_set(&st, NPU_SET_IFM2_BROADCAST))
>                               return -EINVAL;
>                       use_scale = use_ifm2 && (ethosu_is_u65(edev) ?
>                                   (st.ifm2.broadcast & 0x80) :
>                                   (st.ifm2.broadcast == 8));
>                       use_ifm2 = use_ifm2 && !use_scale;
>                       use_ifm = st.ifm.broadcast != 8;
>                       ret = calc_sizes_elemwise(ddev, info, cmd, &st, 
> use_ifm, use_ifm2);
                        
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the changes to
the validation in ethosu_gem_cmdstream_copy_and_validate(), I noticed that
the scale buffer bounds don't seem to be validated for NPU_OP_ELEMENTWISE
operations.

For NPU_OP_ELEMENTWISE commands, the driver parses the broadcast
configuration and sets use_scale = true. However, use_scale is never
passed to calc_sizes_elemwise() and the scale buffer bounds are never
validated.

Could this allow an out-of-bounds NPU memory read if a userspace application
submits a crafted command stream containing an NPU_OP_ELEMENTWISE command
with a broadcast configuration that uses the scale buffer?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=12

Reply via email to