[bug report] mm/zswap: add the flag can_sleep_mapped

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

Hello Tian Tao,

The patch 6753c561f653: "mm/zswap: add the flag can_sleep_mapped"
from Jan 20, 2021, leads to the following static checker warning:

	mm/zswap.c:947 zswap_writeback_entry()
	error: potentially dereferencing uninitialized 'entry'.

mm/zswap.c
   927  static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
   928  {
   929          struct zswap_header *zhdr;
   930          swp_entry_t swpentry;
   931          struct zswap_tree *tree;
   932          pgoff_t offset;
   933          struct zswap_entry *entry;
   934          struct page *page;
   935          struct scatterlist input, output;
   936          struct crypto_acomp_ctx *acomp_ctx;
   937  
   938          u8 *src, *tmp;
   939          unsigned int dlen;
   940          int ret;
   941          struct writeback_control wbc = {
   942                  .sync_mode = WB_SYNC_NONE,
   943          };
   944  
   945          if (!zpool_can_sleep_mapped(pool)) {
   946  
   947                  tmp = kmalloc(entry->length, GFP_ATOMIC);
                                      ^^^^^^^^^^^^^
"entry" uninitialized.

   948                  if (!tmp)
   949                          return -ENOMEM;
   950          }
   951  
   952          /* extract swpentry from data */
   953          zhdr = zpool_map_handle(pool, handle, ZPOOL_MM_RO);
   954          swpentry = zhdr->swpentry; /* here */
   955          tree = zswap_trees[swp_type(swpentry)];
   956          offset = swp_offset(swpentry);
   957  
   958          /* find and ref zswap entry */
   959          spin_lock(&tree->lock);
   960          entry = zswap_entry_find_get(&tree->rbroot, offset);
   961          if (!entry) {
   962                  /* entry was invalidated */
   963                  spin_unlock(&tree->lock);
   964                  zpool_unmap_handle(pool, handle);
   965                  return 0;

memory leak.

   966          }
   967          spin_unlock(&tree->lock);
   968          BUG_ON(offset != entry->offset);
   969  
   970          /* try to allocate swap cache page */
   971          switch (zswap_get_swap_cache_page(swpentry, &page)) {
   972          case ZSWAP_SWAPCACHE_FAIL: /* no memory or invalidate happened */
   973                  ret = -ENOMEM;
   974                  goto fail;
   975  
   976          case ZSWAP_SWAPCACHE_EXIST:
   977                  /* page is already in the swap cache, ignore for now */
   978                  put_page(page);
   979                  ret = -EEXIST;
   980                  goto fail;
   981  
   982          case ZSWAP_SWAPCACHE_NEW: /* page is locked */
   983                  /* decompress */
   984                  acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx);
   985  
   986                  dlen = PAGE_SIZE;
   987                  src = (u8 *)zhdr + sizeof(struct zswap_header);
   988  
   989                  if (!zpool_can_sleep_mapped(pool)) {
   990  
   991                          memcpy(tmp, src, entry->length);
   992                          src = tmp;
   993  
   994                          zpool_unmap_handle(pool, handle);

Why not just do a "src = tmp = kmemdup(src, entry->length, GFP_ATOMIC);
right, here?  That would avoid unnecessary allocations for the other
cases.

This path calls zpool_unmap_handle() and it frees the "tmp" buffer.  The
other fail paths only free the "tmp" buffer but don't call
zpool_unmap_handle() so is that a leak?


   995                  }
   996  
   997                  mutex_lock(acomp_ctx->mutex);
   998                  sg_init_one(&input, src, entry->length);
   999                  sg_init_table(&output, 1);
  1000                  sg_set_page(&output, page, PAGE_SIZE, 0);
  1001                  acomp_request_set_params(acomp_ctx->req, &input, &output, entry->length, dlen);
  1002                  ret = crypto_wait_req(crypto_acomp_decompress(acomp_ctx->req), &acomp_ctx->wait);
  1003                  dlen = acomp_ctx->req->dlen;
  1004                  mutex_unlock(acomp_ctx->mutex);
  1005  
  1006                  BUG_ON(ret);
  1007                  BUG_ON(dlen != PAGE_SIZE);
  1008  
  1009                  /* page is up to date */
  1010                  SetPageUptodate(page);
  1011          }
  1012  
  1013          /* move it to the tail of the inactive list after end_writeback */
  1014          SetPageReclaim(page);
  1015  
  1016          /* start writeback */
  1017          __swap_writepage(page, &wbc, end_swap_bio_write);
  1018          put_page(page);
  1019          zswap_written_back_pages++;
  1020  
  1021          spin_lock(&tree->lock);
  1022          /* drop local reference */
  1023          zswap_entry_put(tree, entry);
  1024  
  1025          /*
  1026          * There are two possible situations for entry here:
  1027          * (1) refcount is 1(normal case),  entry is valid and on the tree
  1028          * (2) refcount is 0, entry is freed and not on the tree
  1029          *     because invalidate happened during writeback
  1030          *  search the tree and free the entry if find entry
  1031          */
  1032          if (entry == zswap_rb_search(&tree->rbroot, offset))
  1033                  zswap_entry_put(tree, entry);
  1034          spin_unlock(&tree->lock);
  1035  
  1036          goto end;
  1037  
  1038          /*
  1039          * if we get here due to ZSWAP_SWAPCACHE_EXIST
  1040          * a load may be happening concurrently.
  1041          * it is safe and okay to not free the entry.
  1042          * if we free the entry in the following put
  1043          * it is also okay to return !0
  1044          */
  1045  fail:
  1046          spin_lock(&tree->lock);
  1047          zswap_entry_put(tree, entry);
  1048          spin_unlock(&tree->lock);
  1049  
  1050  end:
  1051          if (zpool_can_sleep_mapped(pool))
  1052                  zpool_unmap_handle(pool, handle);
  1053          else
  1054                  kfree(tmp);
  1055  
  1056          return ret;
  1057  }

regards,
dan carpenter