* [bug report] mm/zswap: add the flag can_sleep_mapped
@ 2021-01-22 11:08 Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2021-01-22 11:08 UTC (permalink / raw)
To: tiantao6; +Cc: linux-mm
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* [bug report] mm/zswap: add the flag can_sleep_mapped
@ 2021-01-22 11:11 Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2021-01-22 11:11 UTC (permalink / raw)
To: tiantao6; +Cc: linux-mm
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:1322 zswap_frontswap_load()
error: uninitialized symbol 'ret'.
mm/zswap.c
1250 static int zswap_frontswap_load(unsigned type, pgoff_t offset,
1251 struct page *page)
1252 {
1253 struct zswap_tree *tree = zswap_trees[type];
1254 struct zswap_entry *entry;
1255 struct scatterlist input, output;
1256 struct crypto_acomp_ctx *acomp_ctx;
1257 u8 *src, *dst, *tmp;
1258 unsigned int dlen;
1259 int ret;
1260
1261 /* find */
1262 spin_lock(&tree->lock);
1263 entry = zswap_entry_find_get(&tree->rbroot, offset);
1264 if (!entry) {
1265 /* entry was written back */
1266 spin_unlock(&tree->lock);
1267 return -1;
1268 }
1269 spin_unlock(&tree->lock);
1270
1271 if (!entry->length) {
1272 dst = kmap_atomic(page);
1273 zswap_fill_page(dst, entry->value);
1274 kunmap_atomic(dst);
ret = 0; on this path?
1275 goto freeentry;
1276 }
1277
1278 if (!zpool_can_sleep_mapped(entry->pool->zpool)) {
1279
1280 tmp = kmalloc(entry->length, GFP_ATOMIC);
1281 if (!tmp) {
1282 ret = -ENOMEM;
1283 goto freeentry;
1284 }
1285 }
1286
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-01-22 11:11 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-22 11:08 [bug report] mm/zswap: add the flag can_sleep_mapped Dan Carpenter
2021-01-22 11:11 Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).