From daccde3605a826005cee43faee5549eb09b4fafe Mon Sep 17 00:00:00 2001 From: Lingao Meng Date: Wed, 27 Apr 2022 12:23:14 +0800 Subject: [PATCH] drivers: mtd: fix null buffer reference When device not support byte read or readlen less than device block size, will use `dev->buffer` as dst address. `CFGDIOC_FIRSTCONFIG` and `CFGDIOC_NEXTCONFIG` should also malloc buffer before call `mtdconfig_readbytes`. Signed-off-by: Lingao Meng --- drivers/mtd/mtd_config.c | 237 ++++++++++++++++++++++++--------------- 1 file changed, 145 insertions(+), 92 deletions(-) diff --git a/drivers/mtd/mtd_config.c b/drivers/mtd/mtd_config.c index 7a597dc9ecf..6cf2218b1cc 100644 --- a/drivers/mtd/mtd_config.c +++ b/drivers/mtd/mtd_config.c @@ -1496,6 +1496,149 @@ static int mtdconfig_deleteconfig(FAR struct mtdconfig_struct_s *dev, return ret; } +/**************************************************************************** + * Name: mtdconfig_firstconfig + ****************************************************************************/ + +static int mtdconfig_firstconfig(FAR struct mtdconfig_struct_s *dev, + FAR struct config_data_s *pdata) +{ + int ret = -ENOENT; + off_t bytes_to_read; + struct mtdconfig_header_s hdr; + + /* Allocate a temp block buffer */ + + dev->buffer = (FAR uint8_t *)kmm_malloc(dev->blocksize); + if (dev->buffer == NULL) + { + return -ENOMEM; + } + + dev->readoff = mtdconfig_findfirstentry(dev, &hdr); + + /* Test if the config item is valid */ + +#ifdef CONFIG_MTD_CONFIG_NAMED + if (dev->readoff != 0 && + hdr.name[0] != CONFIG_MTD_CONFIG_ERASEDVALUE) +#else + if (dev->readoff != 0 && hdr.id != MTD_ERASED_ID) +#endif + { + /* Perform the read */ + + bytes_to_read = hdr.len; + if (bytes_to_read > pdata->len) + { + bytes_to_read = pdata->len; + } + + ret = mtdconfig_readbytes(dev, dev->readoff + sizeof(hdr), + pdata->configdata, bytes_to_read); + if (ret < 0) + { + goto errout; + } + + /* Set other return data items */ + +#ifdef CONFIG_MTD_CONFIG_NAMED + strcpy(pdata->name, hdr.name); +#else + pdata->id = hdr.id; + pdata->instance = hdr.instance; +#endif + pdata->len = bytes_to_read; + } + else + { + ret = -ENOENT; + } + +errout: + + /* Free the buffer */ + + kmm_free(dev->buffer); + return ret; +} + +/**************************************************************************** + * Name: mtdconfig_nextconfig + ****************************************************************************/ + +static int mtdconfig_nextconfig(FAR struct mtdconfig_struct_s *dev, + FAR struct config_data_s *pdata) +{ + int ret = -ENOENT; + off_t bytes_to_read; + struct mtdconfig_header_s hdr; + + /* Allocate a temp block buffer */ + + dev->buffer = (FAR uint8_t *)kmm_malloc(dev->blocksize); + if (dev->buffer == NULL) + { + return -ENOMEM; + } + + ret = mtdconfig_readbytes(dev, dev->readoff, (FAR uint8_t *)&hdr, + sizeof(hdr)); + if (ret < 0) + { + goto errout; + } + + dev->readoff = mtdconfig_findnextentry(dev, dev->readoff, &hdr, 0); + + /* Test if the config item is valid */ + +#ifdef CONFIG_MTD_CONFIG_NAMED + if (dev->readoff != 0 && + hdr.name[0] != CONFIG_MTD_CONFIG_ERASEDVALUE) +#else + if (dev->readoff != 0 && hdr.id != MTD_ERASED_ID) +#endif + { + /* Test if this is an empty slot */ + + bytes_to_read = hdr.len; + if (bytes_to_read > pdata->len) + { + bytes_to_read = pdata->len; + } + + /* Read the config item data */ + + ret = mtdconfig_readbytes(dev, dev->readoff + sizeof(hdr), + pdata->configdata, bytes_to_read); + if (ret < 0) + { + goto errout; + } + +#ifdef CONFIG_MTD_CONFIG_NAMED + strcpy(pdata->name, hdr.name); +#else + pdata->id = hdr.id; + pdata->instance = hdr.instance; +#endif + pdata->len = bytes_to_read; + } + else + { + ret = -ENOENT; + } + +errout: + + /* Free the buffer */ + + kmm_free(dev->buffer); + return ret; +} + /**************************************************************************** * Name: mtdconfig_ioctl ****************************************************************************/ @@ -1506,8 +1649,6 @@ static int mtdconfig_ioctl(FAR struct file *filep, int cmd, FAR struct inode *inode = filep->f_inode; FAR struct mtdconfig_struct_s *dev = inode->i_private; FAR struct config_data_s *pdata; - struct mtdconfig_header_s hdr; - off_t bytes_to_read; int ret = -ENOTTY; switch (cmd) @@ -1541,47 +1682,7 @@ static int mtdconfig_ioctl(FAR struct file *filep, int cmd, /* Get the the first config item */ pdata = (FAR struct config_data_s *)arg; - dev->readoff = mtdconfig_findfirstentry(dev, &hdr); - - /* Test if the config item is valid */ - -#ifdef CONFIG_MTD_CONFIG_NAMED - if (dev->readoff != 0 && - hdr.name[0] != CONFIG_MTD_CONFIG_ERASEDVALUE) -#else - if (dev->readoff != 0 && hdr.id != MTD_ERASED_ID) -#endif - { - /* Perform the read */ - - bytes_to_read = hdr.len; - if (bytes_to_read > pdata->len) - { - bytes_to_read = pdata->len; - } - - ret = mtdconfig_readbytes(dev, dev->readoff + sizeof(hdr), - pdata->configdata, bytes_to_read); - if (ret < 0) - { - break; - } - - /* Set other return data items */ - -#ifdef CONFIG_MTD_CONFIG_NAMED - strcpy(pdata->name, hdr.name); -#else - pdata->id = hdr.id; - pdata->instance = hdr.instance; -#endif - pdata->len = bytes_to_read; - } - else - { - ret = -ENOENT; - } - + ret = mtdconfig_firstconfig(dev, pdata); break; case CFGDIOC_NEXTCONFIG: @@ -1589,55 +1690,7 @@ static int mtdconfig_ioctl(FAR struct file *filep, int cmd, /* Get the next config item */ pdata = (FAR struct config_data_s *)arg; - - ret = mtdconfig_readbytes(dev, dev->readoff, (FAR uint8_t *)&hdr, - sizeof(hdr)); - if (ret < 0) - { - break; - } - - dev->readoff = mtdconfig_findnextentry(dev, dev->readoff, &hdr, 0); - - /* Test if the config item is valid */ - -#ifdef CONFIG_MTD_CONFIG_NAMED - if (dev->readoff != 0 && - hdr.name[0] != CONFIG_MTD_CONFIG_ERASEDVALUE) -#else - if (dev->readoff != 0 && hdr.id != MTD_ERASED_ID) -#endif - { - /* Test if this is an empty slot */ - - bytes_to_read = hdr.len; - if (bytes_to_read > pdata->len) - { - bytes_to_read = pdata->len; - } - - /* Read the config item data */ - - ret = mtdconfig_readbytes(dev, dev->readoff + sizeof(hdr), - pdata->configdata, bytes_to_read); - if (ret < 0) - { - break; - } - -#ifdef CONFIG_MTD_CONFIG_NAMED - strcpy(pdata->name, hdr.name); -#else - pdata->id = hdr.id; - pdata->instance = hdr.instance; -#endif - pdata->len = bytes_to_read; - } - else - { - ret = -ENOENT; - } - + ret = mtdconfig_nextconfig(dev, pdata); break; case MTDIOC_BULKERASE: