On Fri, Aug 02, 2019 at 03:27:54PM -0700, Stephen Boyd wrote: > Quoting Hung-Te Lin (2019-08-02 01:20:31) > > The VPD implementation from Chromium Vital Product Data project has been > > updated so vpd_decode be easily shared by kernel, firmware and the user > > space utility programs. Also improved value range checks to prevent > > kernel crash due to bad VPD data. > > Please add a Fixes: tag here to fix the commit that introduces the > problem. It would also be nice to get a description of the problem that > this patch is fixing. For example, explaining why the types change from > signed to unsigned. > > > > > Signed-off-by: Hung-Te Lin <[email protected]> > > --- > > drivers/firmware/google/vpd.c | 38 +++++++++------ > > drivers/firmware/google/vpd_decode.c | 69 +++++++++++++++------------- > > drivers/firmware/google/vpd_decode.h | 17 ++++--- > > 3 files changed, 71 insertions(+), 53 deletions(-) > > > > diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c > > index 0739f3b70347..ecf217a7db39 100644 > > --- a/drivers/firmware/google/vpd.c > > +++ b/drivers/firmware/google/vpd.c > > @@ -73,7 +73,7 @@ static ssize_t vpd_attrib_read(struct file *filp, struct > > kobject *kobp, > > * exporting them as sysfs attributes. These keys present in old firmwares > > are > > * ignored. > > * > > - * Returns VPD_OK for a valid key name, VPD_FAIL otherwise. > > + * Returns VPD_DECODE_OK for a valid key name, VPD_DECODE_FAIL otherwise. > > Maybe we should convert these things to use linux conventions instead of > VPD error codes? > > > * > > * @key: The key name to check > > * @key_len: key name length > > @@ -86,14 +86,14 @@ static int vpd_section_check_key_name(const u8 *key, > > s32 key_len) > > c = *key++; > > > > if (!isalnum(c) && c != '_') > > - return VPD_FAIL; > > + return VPD_DECODE_FAIL; > > } > > > > - return VPD_OK; > > + return VPD_DECODE_OK; > > Can you split this rename out into it's own patch. That way we can > confirm that there are no changes due to the rename of the enum. > > > } > > > > -static int vpd_section_attrib_add(const u8 *key, s32 key_len, > > - const u8 *value, s32 value_len, > > +static int vpd_section_attrib_add(const u8 *key, u32 key_len, > > + const u8 *value, u32 value_len, > > void *arg) > > { > > int ret; > > @@ -246,7 +246,7 @@ static int vpd_section_destroy(struct vpd_section *sec) > > > > static int vpd_sections_init(phys_addr_t physaddr) > > { > > - struct vpd_cbmem *temp; > > + struct vpd_cbmem __iomem *temp;
The change to __iomem should also be a separate patch. Guenter
