From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755050AbcCBW0o (ORCPT ); Wed, 2 Mar 2016 17:26:44 -0500 Received: from vps0.lunn.ch ([178.209.37.122]:48124 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753827AbcCBW0n (ORCPT ); Wed, 2 Mar 2016 17:26:43 -0500 Date: Wed, 2 Mar 2016 23:26:39 +0100 From: Andrew Lunn To: Vladimir Zapolskiy Cc: GregKH , srinivas.kandagatla@linaro.org, maxime.ripard@free-electrons.com, wsa@the-dreams.de, broonie@kernel.org, linux-kernel@vger.kernel.org, pantelis.antoniou@konsulko.com, bgolaszewski@baylibre.com Subject: Re: [PATCHv7 6/7] eeprom: 93xx46: extend driver to plug into the NVMEM framework Message-ID: <20160302222639.GC15541@lunn.ch> References: <1456516764-1456-1-git-send-email-andrew@lunn.ch> <1456516764-1456-7-git-send-email-andrew@lunn.ch> <56D7644F.8060601@mleia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56D7644F.8060601@mleia.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > > 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