Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Missing bounds validation for Tile 0's maximum Y extent when checking NHCWB16 (storage == 1) feature matrices. -- commit 55c57d446ca04d4d5fc6125acd2d154b7839c8a4 Author: Rob Herring (Arm) <[email protected]> accel: ethosu: Restrict dynamic IFM2 weights This commit adds validation to reject dynamic-weight convolutions where the kernel is not 1x1. This prevents a crafted kernel from causing the NPU to access memory beyond the validated IFM2 feature map. Cc: [email protected] > diff --git a/drivers/accel/ethosu/ethosu_gem.c > b/drivers/accel/ethosu/ethosu_gem.c > index 632a2352491a0..df26ccd50a96a 100644 > --- a/drivers/accel/ethosu/ethosu_gem.c > +++ b/drivers/accel/ethosu/ethosu_gem.c [ ... ] While reviewing the memory accesses in this file, I noticed a pre-existing issue in feat_matrix_size(): drivers/accel/ethosu/ethosu_gem.c:feat_matrix_size() { ... if (storage == 1) { ret = feat_matrix_check_location(edev, info, st, fm, type, x, 0, c, ofm, max_len); if (ret) return ret; ... } [Severity: Critical] This is a pre-existing issue, but does this NHCWB16 format (storage == 1) validation correctly check the maximum Y coordinate accessed in Tile 0? For storage == 0, the code correctly checks min(y, fm->height[0]). However, for storage == 1, it only probes Tile 0 at y = 0. If a userspace application submits a command stream where the overall operation height y > fm->height[0] and fm->stride_y is a very large value, could the hardware access memory up to base[0] + fm->height[0] * stride_y? Since the validator skips checking y = fm->height[0] for Tile 0, the computed info->region_size appears to be drastically underestimated. Could this underestimation bypass the GEM buffer bounds check, allowing the NPU to read or write out-of-bounds physical memory via DMA? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=19
