Coverity: zblock_alloc(): Memory - illegal accesses
Subsystems (3)
Hello!
This is an experimental semi-automated report about issues detected by Coverity from a scan of next-20221117 as part of the linux-next scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan
You're getting this email because you were associated with the identified lines of code (noted below) that were touched by commits:
Wed Nov 16 16:19:12 2022 -0800
9097e28c25c8 ("mm: add zblock - new allocator for use via zpool API")
Coverity reported the following:
*** CID 1527352: Memory - illegal accesses (OVERRUN) mm/zblock.c:320 in zblock_alloc()
314 }
315 list = &(pool->block_lists[block_type]);
316
317 check: 318 spin_lock(&list->lock); 319 /* check if there are free slots in cache */ vvv CID 1527352: Memory - illegal accesses (OVERRUN) vvv Overrunning array of 10208 bytes at byte offset 10208 by dereferencing pointer "list". 320 block = cache_find_block(list); 321 if (block) 322 goto found; 323 spin_unlock(&list->lock);
324
325 /* not found block with free slots try to allocate new empty block */
If this is a false positive, please let us know so we can mark it as such, or teach the Coverity rules to be smarter. If not, please make sure fixes get into linux-next. :) For patches fixing this, please include these lines (but double-check the "Fixes" first):
Addresses-Coverity-ID: 1527352 ("Memory - illegal accesses")
It looks like block_type is not checked to be < ARRAY_SIZE(block_desc) after exiting the earlier loop, so the access through "list" may be past the end of pool->block_lists.
Thanks for your attention!
Signature · 1 line
Coverity-bot
18.11.2022 01:20, coverity-bot пишет:
Quoted 24 lines
Coverity reported the following:
*** CID 1527352: Memory - illegal accesses (OVERRUN)
mm/zblock.c:320inzblock_alloc()
314 }
315 list = &(pool->block_lists[block_type]);
316317 check: 318 spin_lock(&list->lock); 319 /* check if there are free slots in cache */ vvv CID 1527352: Memory - illegal accesses (OVERRUN) vvv Overrunning array of 10208 bytes at byte offset 10208 by dereferencing pointer "list". 320 block = cache_find_block(list); 321 if (block) 322 goto found; 323 spin_unlock(&list->lock);324
325 /* not found block with free slots try to allocate new empty block */If this is a false positive, please let us know so we can mark it as such, or teach the Coverity rules to be smarter. If not, please make sure fixes get into linux-next. :) For patches fixing this, please include these lines (but double-check the "Fixes" first):
Quoted 8 lines
Reported-by coverity-bot <[email protected]>Addresses-Coverity-ID: 1527352 ("Memory - illegal accesses")
Fixes 9097e28c25c8 ("mm: add zblock - new allocator for use via zpool API")It looks like
block_typeis not checked to be <ARRAY_SIZE(block_desc)after exiting the earlier loop, so the access through "list" may be past the end ofpool->block_lists.
There is no need for this check because it is guaranteed that this code will be executed only if size <= PAGE_SIZE. Since slot_size for the last list even exceeds PAGE_SIZE, block_type will be always valid.
Quoted 29 lines · Ananda Badmaev
18.11.2022 01:20, coverity-bot пишет:
Quoted 28 lines
Coverity reported the following:
*** CID 1527352: Memory - illegal accesses (OVERRUN)
mm/zblock.c:320inzblock_alloc()
314 }
315 list = &(pool->block_lists[block_type]);
316317 check: 318 spin_lock(&list->lock); 319 /* check if there are free slots in cache */ vvv CID 1527352: Memory - illegal accesses (OVERRUN) vvv Overrunning array of 10208 bytes at byte offset 10208 by dereferencing pointer "list". 320 block = cache_find_block(list); 321 if (block) 322 goto found; 323 spin_unlock(&list->lock);324
325 /* not found block with free slots try to allocate new empty block */If this is a false positive, please let us know so we can mark it as such, or teach the Coverity rules to be smarter. If not, please make sure fixes get into linux-next. :) For patches fixing this, please include these lines (but double-check the "Fixes" first):
Reported-by coverity-bot <[email protected]>Addresses-Coverity-ID: 1527352 ("Memory - illegal accesses")
Fixes 9097e28c25c8 ("mm: add zblock - new allocator for use via zpool API")
Quoted 8 lines
Quoted 4 lines
It looks like
block_typeis not checked to be <ARRAY_SIZE(block_desc)after exiting the earlier loop, so the access through "list" may be past the end ofpool->block_lists.There is no need for this check because it is guaranteed that this code will be executed only if size <=
PAGE_SIZE. Sinceslot_sizefor the last list even exceedsPAGE_SIZE,block_typewill be always valid.
Ah-ha, understood. Well, if you do want to catch it if there is ever a typo in the block_desc values (which are not obviously >4096 without sitting down and calculating them), perhaps add:
if (WARN_ON(block_type >= ARRAY_SIZE(block_desc))
return -ENOSPC;
Signature · 1 line
Kees Cook
18.11.2022 19:43, Kees Cook пишет:
Quoted 8 lines
Ah-ha, understood. Well, if you do want to catch it if there is ever a typo in the
block_descvalues (which are not obviously >4096 without sitting down and calculating them), perhaps add:if (
WARN_ON(block_type >= ARRAY_SIZE(block_desc))
return-ENOSPC;
Probably it would be better to add a single check on the largest slot_size >= PAGE_SIZE at pool creation time.