[bug report] MTD: mtdconcat NAND/Sibley support (rev.2)

[Dan Carpenter <dan.carpenter@oracle.com>]

[ I was just going through these warnings and noticed that it had been
  eight years since I reported this.  :P It still looks valid.
  -dan ]

Hi MTD developers,

The patch e8d32937d9f2: "MTD: mtdconcat NAND/Sibley support (rev.2)"
from May 17, 2006, leads to the following Smatch static checker
warning:

	drivers/mtd/mtdconcat.c:231 concat_writev()
	warn: potentially one past the end of array 'vecs_copy[entry_high]'

drivers/mtd/mtdconcat.c
    184 static int
    185 concat_writev(struct mtd_info *mtd, const struct kvec *vecs,
    186                 unsigned long count, loff_t to, size_t * retlen)
    187 {
    188         struct mtd_concat *concat = CONCAT(mtd);
    189         struct kvec *vecs_copy;
    190         unsigned long entry_low, entry_high;
    191         size_t total_len = 0;
    192         int i;
    193         int err = -EINVAL;
    194 
    195         /* Calculate total length of data */
    196         for (i = 0; i < count; i++)
    197                 total_len += vecs[i].iov_len;
    198 
    199         /* Check alignment */
    200         if (mtd->writesize > 1) {
    201                 uint64_t __to = to;
    202                 if (do_div(__to, mtd->writesize) || (total_len % mtd->writesize))
    203                         return -EINVAL;
    204         }
    205 
    206         /* make a copy of vecs */
    207         vecs_copy = kmemdup(vecs, sizeof(struct kvec) * count, GFP_KERNEL);
                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    208         if (!vecs_copy)
    209                 return -ENOMEM;
    210 
    211         entry_low = 0;
    212         for (i = 0; i < concat->num_subdev; i++) {
    213                 struct mtd_info *subdev = concat->subdev[i];
    214                 size_t size, wsize, retsize, old_iov_len;
    215 
    216                 if (to >= subdev->size) {
    217                         to -= subdev->size;
    218                         continue;
    219                 }
    220 
    221                 size = min_t(uint64_t, total_len, subdev->size - to);
    222                 wsize = size; /* store for future use */
    223 
    224                 entry_high = entry_low;
    225                 while (entry_high < count) {
                               ^^^^^^^^^^^^^^^^^^
If "entry_high == count"


    226                         if (size <= vecs_copy[entry_high].iov_len)
    227                                 break;
    228                         size -= vecs_copy[entry_high++].iov_len;
    229                 }
    230 
--> 231                 old_iov_len = vecs_copy[entry_high].iov_len;

Then this is an off by one.

    232                 vecs_copy[entry_high].iov_len = size;
    233 
    234                 err = mtd_writev(subdev, &vecs_copy[entry_low],
    235                                  entry_high - entry_low + 1, to, &retsize);
    236 
    237                 vecs_copy[entry_high].iov_len = old_iov_len - size;
    238                 vecs_copy[entry_high].iov_base += size;
    239 
    240                 entry_low = entry_high;
    241 
    242                 if (err)
    243                         break;
    244 
    245                 *retlen += retsize;
    246                 total_len -= wsize;
    247 
    248                 if (total_len == 0)
    249                         break;
    250 
    251                 err = -EINVAL;
    252                 to = 0;
    253         }
    254 
    255         kfree(vecs_copy);
    256         return err;
    257 }

regards,
dan carpenter

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