[bug report] mm: add zblock - new allocator for use via zpool API

[Dan Carpenter <error27@gmail.com>]

Hello Ananda,

The patch 9097e28c25c8: "mm: add zblock - new allocator for use via
zpool API" from Nov 4, 2022, leads to the following Smatch static
checker warning:

mm/zblock.c:341 zblock_alloc() error: buffer overflow 'block_desc' 29 <= 29 (assuming for loop doesn't break)
mm/zblock.c:165 cache_insert_block() error: uninitialized symbol 'min_index'.
mm/zblock.c:412 zblock_reclaim_block() warn: always true condition '(block_type >= 0) => (0-u64max >= 0)'

mm/zblock.c
    297 static int zblock_alloc(struct zblock_pool *pool, size_t size, gfp_t gfp,
    298                         unsigned long *handle)
    299 {
    300         unsigned int block_type, slot;
    301         struct zblock_block *block;
    302         struct block_list *list;
    303 
    304         if (!size)
    305                 return -EINVAL;
    306 
    307         if (size > PAGE_SIZE)
    308                 return -ENOSPC;
    309 
    310         /* find basic block type with suitable slot size */
    311         for (block_type = 0; block_type < ARRAY_SIZE(block_desc); block_type++) {
    312                 if (size <= block_desc[block_type].slot_size)
    313                         break;
    314         }

"size" is always <= PAGE_SIZE.  Is PAGE_SIZE always 4k?  If so then this
code is fine.  Smatch is bad at handling arrays.

If we don't hit the break then this code has an issue.

    315         list = &(pool->block_lists[block_type]);
    316 
    317 check:

[ Snip ] Similar thing, with breaking from loops:

   150  static void cache_insert_block(struct zblock_block *block, struct block_list *list)
   151  {
   152          unsigned int i, min_free_slots, min_index;
   153  
   154          min_free_slots = MAX_SLOTS;
   155          for (i = 0; i < BLOCK_CACHE_SIZE; i++) {
   156                  if (!list->block_cache[i] || !(list->block_cache[i])->free_slots) {
   157                          list->block_cache[i] = block;
   158                          return;
   159                  }
   160                  if ((list->block_cache[i])->free_slots < min_free_slots) {
   161                          min_free_slots = (list->block_cache[i])->free_slots;
   162                          min_index = i;

Smatch cannot figure out if this condition *must* be true.

   163                  }
   164          }
   165          list->block_cache[min_index] = block;
                                  ^^^^^^^^^
Smatch says this can be uninitialized.

   166  }

[ snip ]

   405  static int zblock_reclaim_block(struct zblock_pool *pool)
   406  {
   407          struct zblock_block *block;
   408          struct block_list *list;
   409          unsigned long handle, block_type, slot;
                ^^^^^^^^^^^^^         ^^^^^^^^^^

   410          int ret, i, reclaimed;
   411  
   412          /* start with list storing blocks with the worst compression and try
   413           * to evict the first added (oldest) block in this list
   414           */
   415          for (block_type = ARRAY_SIZE(block_desc) - 1; block_type >= 0; --block_type) {
                                                              ^^^^^^^^^^^^^^^
The condition is always true.  Just declare "block_type" as an int.

   416                  list = &(pool->block_lists[block_type]);
   417                  spin_lock(&list->lock);
   418  
   419                  /* find the oldest block in list */
   420                  block = list_last_entry(&list->head, struct zblock_block, block_node);
   421  
   422                  if (!block) {




regards,
dan carpenter