[bug report] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver

[bug report] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver

[dan.carpenter]

Hello Christophe Kerello,

The patch 2cd457f328c1: "mtd: rawnand: stm32_fmc2: add STM32 FMC2
NAND flash controller driver" from Dec 14, 2018, leads to the
following static checker warning:

	drivers/mtd/nand/raw/stm32_fmc2_nand.c:350 stm32_fmc2_nfc_select_chip()
	error: buffer overflow 'nfc->data_phys_addr' 2 <= 2

drivers/mtd/nand/raw/stm32_fmc2_nand.c
   334  static int stm32_fmc2_nfc_select_chip(struct nand_chip *chip, int chipnr)
   335  {
   336          struct stm32_fmc2_nfc *nfc = to_stm32_nfc(chip->controller);
   337          struct stm32_fmc2_nand *nand = to_fmc2_nand(chip);
   338          struct dma_slave_config dma_cfg;
   339          int ret;
   340  
   341          if (nand->cs_used[chipnr] == nfc->cs_sel)
   342                  return 0;
   343  
   344          nfc->cs_sel = nand->cs_used[chipnr];
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

   345          stm32_fmc2_nfc_setup(chip);
   346          stm32_fmc2_nfc_timings_init(chip);
   347  
   348          if (nfc->dma_tx_ch && nfc->dma_rx_ch) {
   349                  memset(&dma_cfg, 0, sizeof(dma_cfg));
   350                  dma_cfg.src_addr = nfc->data_phys_addr[nfc->cs_sel];

The ->data_phys_addr[] array has FMC2_MAX_CE elements.

   351                  dma_cfg.dst_addr = nfc->data_phys_addr[nfc->cs_sel];
   352                  dma_cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
   353                  dma_cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
   354                  dma_cfg.src_maxburst = 32;
   355                  dma_cfg.dst_maxburst = 32;
   356  
   357                  ret = dmaengine_slave_config(nfc->dma_tx_ch, &dma_cfg);

[ snip ]

  1741  static int stm32_fmc2_nfc_parse_child(struct stm32_fmc2_nfc *nfc,
  1742                                        struct device_node *dn)
  1743  {
  1744          struct stm32_fmc2_nand *nand = &nfc->nand;
  1745          u32 cs;
  1746          int ret, i;
  1747  
  1748          if (!of_get_property(dn, "reg", &nand->ncs))
  1749                  return -EINVAL;
  1750  
  1751          nand->ncs /= sizeof(u32);
  1752          if (!nand->ncs) {
  1753                  dev_err(nfc->dev, "invalid reg property size\n");
  1754                  return -EINVAL;
  1755          }
  1756  
  1757          for (i = 0; i < nand->ncs; i++) {
  1758                  ret = of_property_read_u32_index(dn, "reg", i, &cs);
  1759                  if (ret) {
  1760                          dev_err(nfc->dev, "could not retrieve reg property: %d\n",
  1761                                  ret);
  1762                          return ret;
  1763                  }
  1764  
  1765                  if (cs > FMC2_MAX_CE) {

Which suggests that this should be >= FMC2_MAX_CE to prevent an off by
one.

  1766                          dev_err(nfc->dev, "invalid reg value: %d\n", cs);
  1767                          return -EINVAL;
  1768                  }
  1769  
  1770                  if (nfc->cs_assigned & BIT(cs)) {
  1771                          dev_err(nfc->dev, "cs already assigned: %d\n", cs);
  1772                          return -EINVAL;
  1773                  }
  1774  
  1775                  nfc->cs_assigned |= BIT(cs);
  1776                  nand->cs_used[i] = cs;
                        ^^^^^^^^^^^^^^^^^^^^^
  1777          }

regards,
dan carpenter

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [bug report] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver

[Christophe Kerello]

Hi Dan,

On 7/20/20 2:37 PM, dan.carpenter@oracle.com wrote:
> 1741  static int stm32_fmc2_nfc_parse_child(struct stm32_fmc2_nfc *nfc,
>    1742                                        struct device_node *dn)
>    1743  {
>    1744          struct stm32_fmc2_nand *nand = &nfc->nand;
>    1745          u32 cs;
>    1746          int ret, i;
>    1747
>    1748          if (!of_get_property(dn, "reg", &nand->ncs))
>    1749                  return -EINVAL;
>    1750
>    1751          nand->ncs /= sizeof(u32);
>    1752          if (!nand->ncs) {
>    1753                  dev_err(nfc->dev, "invalid reg property size\n");
>    1754                  return -EINVAL;
>    1755          }
>    1756
>    1757          for (i = 0; i < nand->ncs; i++) {
>    1758                  ret = of_property_read_u32_index(dn, "reg", i, &cs);
>    1759                  if (ret) {
>    1760                          dev_err(nfc->dev, "could not retrieve reg property: %d\n",
>    1761                                  ret);
>    1762                          return ret;
>    1763                  }
>    1764
>    1765                  if (cs > FMC2_MAX_CE) {
> 
> Which suggests that this should be >= FMC2_MAX_CE to prevent an off by
> one.

Thanks for reporting this issue.
A patch has been sent to solve it. 
(https://patchwork.ozlabs.org/project/linux-mtd/patch/1595325127-32693-1-git-send-email-christophe.kerello@st.com/)

Regards,
Christophe Kerello.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/