[PATCH v3 03/10] mtd: intel-dg: implement access functions

Usyskin, Alexander alexander.usyskin at intel.com
Thu Dec 19 02:39:37 PST 2024


> > +
> > +static ssize_t idg_nvm_rewrite_partial(struct intel_dg_nvm *nvm, loff_t to,
> > +				       loff_t offset, size_t len, const u32
> *newdata)
> > +{
> > +	u32 data = idg_nvm_read32(nvm, to);
> > +
> > +	if (idg_nvm_error(nvm))
> > +		return -EIO;
> > +
> > +	memcpy((u8 *)&data + offset, newdata, len);
> 
> I'm a bit concerned with the usage of len here without any check...
> 

This is an internal helper; we can rely on caller to do things right.

> > +
> > +	idg_nvm_write32(nvm, to, data);
> > +	if (idg_nvm_error(nvm))
> > +		return -EIO;
> > +
> > +	return len;
> > +}
> > +
> > +__maybe_unused
> > +static ssize_t idg_write(struct intel_dg_nvm *nvm, u8 region,
> > +			 loff_t to, size_t len, const unsigned char *buf)
> > +{
> > +	size_t i;
> > +	size_t len8;
> > +	size_t len4;
> > +	size_t to4;
> > +	size_t to_shift;
> > +	size_t len_s = len;
> > +	ssize_t ret;
> > +
> > +	idg_nvm_set_region_id(nvm, region);
> > +
> > +	to4 = ALIGN_DOWN(to, sizeof(u32));
> > +	to_shift = min(sizeof(u32) - ((size_t)to - to4), len);
> > +	if (to - to4) {
> 
> As well the 'to'...
> 
> I was even trying to review all 3 patches together, 3, 4, and 5,
> but there's too much indirection on those values and no checks
> in any place...
> 

These are callbacks for mtd framework. 
The mtd checks sanity of values before passing to callbacks, like:
https://elixir.bootlin.com/linux/v6.13-rc3/source/drivers/mtd/mtdcore.c#L1570

Should we double-check input here?


- - 
Thanks,
Sasha




More information about the linux-mtd mailing list