Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Inconsistent extraction of the 'storage' configuration bitfield between `feat_matrix_size` and `feat_matrix_length` allows an attacker to bypass bounds checking for `base[3]` on U65 devices, leading to arbitrary out-of-bounds memory accesses by the NPU. -- commit bea32820db5d7b74645ac247f33a7633b6b22166 Author: Rob Herring (Arm) <[email protected]> accel: ethosu: Validate OFM transpose This commit permutes the output endpoint before validating its tile and stride accesses on U85 devices, while rejecting reserved transpose encodings. > diff --git a/drivers/accel/ethosu/ethosu_gem.c > b/drivers/accel/ethosu/ethosu_gem.c > index a042e650f626a..ad36fb8b3b302 100644 > --- a/drivers/accel/ethosu/ethosu_gem.c > +++ b/drivers/accel/ethosu/ethosu_gem.c > @@ -283,6 +329,9 @@ static int feat_matrix_size(struct ethosu_device *edev, > int ret; > > *max_len = 0; > + ret = feat_matrix_permute(edev, fm, &x, &y, &c, ofm); > + if (ret) > + return ret; > > if (ethosu_is_u65(edev) || storage == 0) { [Severity: Critical] This isn't a bug introduced by this patch, but does this inconsistent extraction of the storage bitfield lead to missing bounds checks for the 4th tile on U65 devices? In feat_matrix_size(), storage is correctly forced to 0 for U65 hardware: drivers/accel/ethosu/ethosu_gem.c:feat_matrix_size() { u32 storage = ethosu_is_u65(edev) ? 0 : fm->precision >> 14; ... } This ensures the loop checks all 4 possible tile coordinates. However, feat_matrix_length() extracts it directly from the precision bits without checking if the hardware is U65: drivers/accel/ethosu/ethosu_gem.c:feat_matrix_length() { u32 element_size, storage = fm->precision >> 14; ... } If a user provides a command stream with bits 14-15 set to 1 (indicating storage = 1) and sets a large, out-of-bounds offset for base[3], feat_matrix_size() forces storage = 0 and delegates address computation to feat_matrix_length(). Because feat_matrix_length() uses the unmasked storage = 1 value, its tile computation logic will only ever produce tile = 1 or tile = 2. As a result, the 4th tile (base[3]) is never visited or verified against the GEM buffer size. The previous commit "accel: ethosu: Validate all feature map tiles" attempted to fix U65 tile validation, but left the direct bitwise extraction in feat_matrix_length() intact. Since U65 hardware natively uses 2x2 tiling and ignores the storage bits, could this allow the NPU DMA engine to access arbitrary physical memory outside the provided GEM buffers when evaluating the true coordinates? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
