Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Flawed buffer bounds calculation for RESIZE operations ignores the 
kernel footprint, allowing the NPU to read out of bounds.
--

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

accel: ethosu: Validate resize operations

This commit adds validation for resize operations by explicitly requiring and
verifying the scale, offset, and step registers. It also computes a
conservative coordinate bound when validating the input feature map.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c 
> b/drivers/accel/ethosu/ethosu_gem.c
> index 8114447891b25..67bee96129341 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -619,6 +631,96 @@ calc_acc_input_size(struct drm_device *ddev,
[ ... ]
> +static int calc_sizes_resize(struct drm_device *ddev,
> +                          struct ethosu_validated_cmdstream_info *info,
> +                          struct cmd_state *st)
> +{
> +     struct ethosu_device *edev = to_ethosu_device(ddev);
> +     u32 ifm_width, ifm_height;
> +     u64 len;
> +     int ret;
[ ... ]
> +     ret = resize_axis_size(st, 0, st->ofm.width, st->ofm_blk_width,
> +                            &ifm_width);
> +     if (ret)
> +             return ret;
> +     ret = resize_axis_size(st, 1, st->ofm.height[2], st->ofm_blk_height,
> +                            &ifm_height);
> +     if (ret)
> +             return ret;
> +
> +     ret = feat_matrix_size(edev, info, st, &st->ifm, FEAT_MATRIX_IFM,
> +                            max(ifm_width, (u32)st->ifm.width),
> +                            max(ifm_height, (u32)st->ifm.height[2]), 
> st->ifm.depth,
                               ^^^^

[Severity: High]
Does this calculation underestimate the required buffer size by ignoring the
kernel footprint?

In calc_sizes_resize(), the starting input feature map (IFM) coordinate
for the last output pixel is calculated by resize_axis_size() as ifm_width.
Because the NPU applies a kernel of size st->ifm.width + 1 at this
coordinate, the true maximum IFM coordinate accessed should be
ifm_width + st->ifm.width.

By combining the starting coordinate and kernel width with max() instead of
addition, the bound checks max(ifm_width, st->ifm.width). This allows a
malicious command stream with a large kernel width to read memory past the
validated input feature map buffer.

Could we use addition here to correctly account for the kernel footprint?

> +                            false, &len);
> +     dev_dbg(ddev->dev, "op %d: IFM:%d:0x%llx-0x%llx\n", NPU_OP_RESIZE,
> +             st->ifm.region, st->ifm.base[0], len);
> +     if (ret)
> +             return ret;

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

Reply via email to