On Fri, Jul 17, 2026 at 10:58:10AM +0000, [email protected] wrote:
> > + regstate_size = FIELD_GET(CLA_SRSTATE_0_REGSTATE, srstate[0]);
> > + if (WARN_ON(*regstate_off + regstate_size * 8 > dev->iassizes))
> > + return -ENOSPC;
>
> [Severity: High]
> Does this bounds check correctly handle the units for *regstate_off?
>
> *regstate_off is incremented by regstate_size (which is a register count),
> so it is also a register count.
>
> If dev->iassizes is in bytes, this check improperly adds a register count
> (*regstate_off) to a byte count (regstate_size * 8) and compares it against
> a byte count.
Yes this should be WARN_ON((*regstate_off + regstate_size) * 8 > dev->iassizes)
> > +int cla_regs_switch_out(struct cla_dev *dev, struct cla_regs *regs,
> > + bool save_regs)
> > +{
> [ ... ]
> > + if (save_regs) {
> > + for (i = 0; i < CLA_NUM_DATA_REGS; i++)
> > + regs->data[i] = cla_reg_read(dev, CLA_REG_DATA(i));
> > +
> > + cla_for_each_accid(dev, accid) {
> > + ret = cla_regs_save_accel(dev, accid, regs,
> > ®state_off);
> > + if (ret)
> > + return ret;
>
> [Severity: Medium]
> If cla_regs_save_accel() fails here, the function returns without clearing
> regs->accel_valid.
>
> Since regs is allocated once per domain and reused, accel_valid might
> already be true from a previous successful save. Returning an error here
> leaves the structure partially overwritten (a mix of old and new accelerator
> states) but still marked as valid.
>
> If the caller handles this switch-out failure non-fatally and retries the
> context later, will cla_regs_switch_in() restore this corrupted state to
> the hardware?
Switch-out failure is fatal, requires at least module reload
> > + }
> > +
> > + regs->accel_valid = true;
> > + }
> [ ... ]
> > +struct cla_regs **cla_regs_alloc_domain(struct cla_domain *domain)
> > +{
> [ ... ]
> > + for (i = 0; i < domain->nr_devs; i++) {
> > + struct cla_regs *regs;
> > + struct cla_dev *dev = domain->devs[i];
> > +
> > + /*
> > + * The regs structures are only ever accessed from the CLA
> > + * device's CPU, so try to allocate them on the right NUMA node
> > + */
> > + size = sizeof(*regs) + dev->iassizes;
>
> [Severity: High]
> If dev->iassizes represents a register count rather than a byte count,
> should it be scaled by sizeof(u64) here?
iassizes is a byte count
