Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] A 32-bit integer overflow when calculating `reg_group + 8` causes out-of-bounds memory access on the `regs` buffer and an infinite loop for high register indices. - [High] The safety bounds check `regidx + nregs > max_regidx` is vulnerable to a 64-bit integer overflow, allowing massive `nregs` values to bypass the limit check. --
--- Patch [2]: [RFC PATCH v1 2/8] misc/arm-cla: Add launch operation helpers --- commit dc6365dd6ad591fc5563520a7266898c01430558 Author: Jean-Philippe Brucker <[email protected]> misc/arm-cla: Add launch operation helpers CLA commands are issued by programming the LAUNCH register and polling LRESP. This patch adds a common launch helper to perform this sequence and translates response codes into Linux errors, along with register read and write support. > diff --git a/drivers/misc/arm-cla/cla-ops.c b/drivers/misc/arm-cla/cla-ops.c > new file mode 100644 > index 0000000000000..d594344a2eb45 > --- /dev/null > +++ b/drivers/misc/arm-cla/cla-ops.c [ ... ] > +static int cla_op_access_reg(struct cla_dev *dev, u8 op, > + enum cla_launch_data_mode data_mode, > + unsigned int accid, unsigned int regidx, > + size_t nregs, u64 *regs) > +{ > + int ret = -EINVAL; > + unsigned long max_regidx; > + struct cla_launch launch = { > + .op = op, > + .accid = accid, > + .data_mode = data_mode, > + }; > + > + switch (op) { > + case CLA_LAUNCH_OP_REGREAD: > + case CLA_LAUNCH_OP_REGWRITE: > + max_regidx = 0x100000000; > + break; > + default: > + WARN_ON(1); > + return -EINVAL; > + } > + > + if (WARN_ON(regidx + nregs > max_regidx)) [Severity: High] Does this check adequately protect against integer overflow? If nregs is very large (for example, (size_t)-1), the addition of regidx and nregs can overflow size_t. This would result in a small positive value that bypasses the max_regidx limit. Could this be rewritten to avoid wrap-around, perhaps by checking if nregs is greater than max_regidx - regidx? > + return -EINVAL; > + > + /* 1 to 8 registers accessed at a time, within the same 8-reg group */ > + while (nregs > 0) { > + unsigned int reg_group = ALIGN_DOWN(regidx, 8); > + unsigned int max_reg = min(regidx + nregs, reg_group + 8); [Severity: High] Can reg_group + 8 overflow? Since regidx and reg_group are 32-bit unsigned integers, if a caller passes a register index near the 32-bit maximum (for example, 0xFFFFFFF8) and a small nregs value, reg_group + 8 will wrap around to 0. If max_reg evaluates to 0, ndata becomes 0 - regidx, which can yield a value larger than the original nregs. Will this cause out-of-bounds memory accesses on the regs buffer when passed to cla_op_launch(), and an infinite loop due to underflowing nregs? > + unsigned int ndata = max_reg - regidx; > + > + launch.ndata_m1 = ndata - 1; > + launch.regidx = regidx; > + launch.data = regs; > + > + ret = cla_op_launch(dev, &launch); > + if (ret) > + break; > + > + regidx += ndata; > + regs += ndata; > + nregs -= ndata; > + } > + > + return ret; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
