Messages in this thread | | | Date | Wed, 2 Mar 2016 23:26:39 +0100 | From | Andrew Lunn <> | Subject | Re: [PATCHv7 6/7] eeprom: 93xx46: extend driver to plug into the NVMEM framework |
| |
> > static ssize_t > > -eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj, > > - struct bin_attribute *bin_attr, > > - char *buf, loff_t off, size_t count) > > +eeprom_93xx46_read(struct eeprom_93xx46_dev *edev, char *buf, > > + unsigned off, size_t count) > > { > > - struct eeprom_93xx46_dev *edev; > > - struct device *dev; > > ssize_t ret = 0; > > > > - dev = kobj_to_dev(kobj); > > - edev = dev_get_drvdata(dev); > > + if (unlikely(off >= edev->size)) > > + return 0; > > + if ((off + count) > edev->size) > > + count = edev->size - off; > > + if (unlikely(!count)) > > + return count; > > > > I'm scratching my head, do you want to kind of revert > the change https://lkml.org/lkml/2015/7/26/89 ? Why?
Hi Vladimir
I had not noticed you had removed this.
> If you know regmap_config.max_register, then all necessary > boundary checks can be done inside NVMEM core.
You don't have to use NVMEM, you could use the regmap directly. It is a public API. Also, during implementation, i did manage to get out of bounds read passed into the drivers and they caused a crash. That might of been AT24, i don't remember, but verifying is better than possible crashing.
> > +/* > > + * Provide a regmap interface, which is registered with the NVMEM > > + * framework > > +*/ > > +static int eeprom_93xx46_regmap_read(void *context, const void *reg, > > + size_t reg_size, void *val, > > + size_t val_size) > > +{ > > + struct eeprom_93xx46_dev *eeprom_93xx46 = context; > > + off_t offset = *(u32 *)reg; > > + int err; > > + > > + err = eeprom_93xx46_read(eeprom_93xx46, val, offset, val_size); > > + if (err) > > + return err; > > + return 0; > > return eeprom_93xx46_read(eeprom_93xx46, val, offset, val_size);
As i've said a few times now to a few different people reviewing these patches, regmap wants either an error code or 0.
Andrew
| |