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