Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] dma_params_valid() incorrectly returns U64_MAX in a boolean function, which evaluates to true and bypasses missing DMA parameter validation. --
commit 6f2462930489d04d8fd80b48b9dfe3672578205e Author: Rob Herring (Arm) <[email protected]> accel: ethosu: Track command stream register setup The current method of tracking command stream state initializing state tracking to illegal values and assuming unaccessed registers are 0 is proving inadequate with additional validation. Instead, track all the registers in a bitmap as the register address space is fairly small. > diff --git a/drivers/accel/ethosu/ethosu_gem.c > b/drivers/accel/ethosu/ethosu_gem.c > index 408b93350dd79..8a44a5d89bce6 100644 > --- a/drivers/accel/ethosu/ethosu_gem.c > +++ b/drivers/accel/ethosu/ethosu_gem.c [ ... ] > @@ -158,13 +185,54 @@ static u64 cmd_to_addr(u32 *cmd) [ ... ] > +static bool dma_params_valid(struct ethosu_device *edev, struct cmd_state > *st, > + const struct dma_state *dma_st, > + const struct dma *dma, > + u16 region_cmd, u16 addr_cmd) > +{ > + s8 mode = dma->mode; > + > + if (!cmd_state_reg_is_set(st, region_cmd) || > + !cmd_state_reg_is_set(st, addr_cmd) || > + !cmd_state_reg_is_set(st, NPU_SET_DMA0_LEN) || mode < 0 || mode > 2) > + return false; > + > + if (mode >= 1 && > + !cmd_state_reg_is_set(st, dma_use_src_stride(edev, dma_st, dma) ? > + NPU_SET_DMA0_SRC_STRIDE0 : > + NPU_SET_DMA0_DST_STRIDE0)) > + return U64_MAX; [Severity: High] Since dma_params_valid() returns a bool, does returning U64_MAX here get converted to true? If an attacker intentionally omits the stride command in a malicious stream, could this evaluate as true and inadvertently bypass the validation check, potentially leading to out-of-bounds DMA accesses using retained hardware stride values? > + if (mode == 2 && > + !cmd_state_reg_is_set(st, dma_use_src_stride(edev, dma_st, dma) ? > + NPU_SET_DMA0_SRC_STRIDE1 : > + NPU_SET_DMA0_DST_STRIDE1)) > + return U64_MAX; [Severity: High] Does returning U64_MAX here also inadvertently evaluate to true? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
