[bug report] mtd: spi-nor: add the framework for SPI NOR

[bug report] mtd: spi-nor: add the framework for SPI NOR

[Dan Carpenter]

Hello MTD devs,

The patch b199489d37b2: "mtd: spi-nor: add the framework for SPI NOR"
from Feb 24, 2014, leads to the following Smatch static checker
warning:

	drivers/mtd/spi-nor/sst.c:157 sst_nor_write()
	warn: 'len - 1' negative user limit promoted to high

drivers/mtd/spi-nor/sst.c
    120 static int sst_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
                                                                  ^^^^^^^^^^
"len" is unsigned long.
    121                          size_t *retlen, const u_char *buf)
    122 {
    123         struct spi_nor *nor = mtd_to_spi_nor(mtd);
    124         size_t actual = 0;
                ^^^^^^^^^^^^^^^^^^
So is "actual".

    125         int ret;
    126 
    127         dev_dbg(nor->dev, "to 0x%08x, len %zd\n", (u32)to, len);
    128 
    129         ret = spi_nor_lock_and_prep(nor);
    130         if (ret)
    131                 return ret;
    132 
    133         ret = spi_nor_write_enable(nor);
    134         if (ret)
    135                 goto out;
    136 
    137         nor->sst_write_second = false;
    138 
    139         /* Start write from odd address. */
    140         if (to % 2) {
    141                 nor->program_opcode = SPINOR_OP_BP;
    142 
    143                 /* write one byte. */
    144                 ret = spi_nor_write_data(nor, to, 1, buf);
    145                 if (ret < 0)
    146                         goto out;
    147                 WARN(ret != 1, "While writing 1 byte written %i bytes\n", ret);
    148                 ret = spi_nor_wait_till_ready(nor);
    149                 if (ret)
    150                         goto out;
    151 
    152                 to++;
    153                 actual++;
    154         }
    155 
    156         /* Write out most of the data here. */
--> 157         for (; actual < len - 1; actual += 2) {
                       ^^^^^^^^^^^^^^^^
I have looked at the call tree and it definitely seems like "len" can
be zero.  So the -1 leads to an integer underflow and instead of being a
no-op, this loops to ULONG_MAX.

    158                 nor->program_opcode = SPINOR_OP_AAI_WP;
    159 
    160                 /* write two bytes. */
    161                 ret = spi_nor_write_data(nor, to, 2, buf + actual);
    162                 if (ret < 0)
    163                         goto out;
    164                 WARN(ret != 2, "While writing 2 bytes written %i bytes\n", ret);
    165                 ret = spi_nor_wait_till_ready(nor);
    166                 if (ret)
    167                         goto out;
    168                 to += 2;
    169                 nor->sst_write_second = true;
    170         }
    171         nor->sst_write_second = false;
    172 
    173         ret = spi_nor_write_disable(nor);
    174         if (ret)
    175                 goto out;
    176 
    177         ret = spi_nor_wait_till_ready(nor);
    178         if (ret)
    179                 goto out;
    180 
    181         /* Write out trailing byte if it exists. */
    182         if (actual != len) {
    183                 ret = spi_nor_write_enable(nor);
    184                 if (ret)
    185                         goto out;
    186 
    187                 nor->program_opcode = SPINOR_OP_BP;
    188                 ret = spi_nor_write_data(nor, to, 1, buf + actual);
    189                 if (ret < 0)
    190                         goto out;
    191                 WARN(ret != 1, "While writing 1 byte written %i bytes\n", ret);
    192                 ret = spi_nor_wait_till_ready(nor);
    193                 if (ret)
    194                         goto out;
    195 
    196                 actual += 1;
    197 
    198                 ret = spi_nor_write_disable(nor);
    199         }
    200 out:
    201         *retlen += actual;
    202         spi_nor_unlock_and_unprep(nor);
    203         return ret;
    204 }

regards,
dan carpenter

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