On 10/23/19 7:06 AM, Dan Carpenter wrote:
> Hello Ajay Joshi,
>
> The patch dd85b4922de1: "null_blk: return fixed zoned reads > write
> pointer" from Oct 17, 2019, leads to the following static checker
> warning:
>
> drivers/block/null_blk_zoned.c:91 null_zone_valid_read_len()
> warn: uncapped user index 'dev->zones[null_zone_no(dev, sector)]'
>
> drivers/block/null_blk_zoned.c
> 87 size_t null_zone_valid_read_len(struct nullb *nullb,
> 88 sector_t sector, unsigned int len)
> 89 {
> 90 struct nullb_device *dev = nullb->dev;
> 91 struct blk_zone *zone = &dev->zones[null_zone_no(dev, sector)];
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> 92 unsigned int nr_sectors = len >> SECTOR_SHIFT;
> 93
> 94 /* Read must be below the write pointer position */
> 95 if (zone->type == BLK_ZONE_TYPE_CONVENTIONAL ||
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> 96 sector + nr_sectors <= zone->wp)
> 97 return len;
> 98
> 99 if (sector > zone->wp)
> ^^^^^^^^^^^^^^^^^
>
> Smatch complains about "sector" being from the untrusted all the time
> and I kind of just ignore it these days. But here it looks like we're
> checking "sector" after we already used it so that seems very suspicious.
> It feels like "sector > zone->wp" should come at the very start of the
> function.
>
> 100 return 0;
> 101
> 102 return (zone->wp - sector) << SECTOR_SHIFT;
> 103 }
>
Ajay, please take a look at this and send in a patch if appropriate.
--
Jens Axboe
