* Coverity: do_migrate_range(): Error handling issues
@ 2021-06-08 18:05 coverity-bot
2021-06-08 19:35 ` Linus Torvalds
0 siblings, 1 reply; 4+ messages in thread
From: coverity-bot @ 2021-06-08 18:05 UTC (permalink / raw)
To: Shakeel Butt
Cc: Andrew Morton, Linus Torvalds, Johannes Weiner,
Gustavo A. R. Silva, linux-next
Hello!
This is an experimental semi-automated report about issues detected by
Coverity from a scan of next-20210608 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:
Tue Dec 15 12:13:39 2020 -0800
013339df116c ("mm/rmap: always do TTU_IGNORE_ACCESS")
Coverity reported the following:
*** CID 1505146: Error handling issues (CHECKED_RETURN)
/mm/memory_hotplug.c: 1422 in do_migrate_range()
1416 * the unmap as the catch all safety net).
1417 */
1418 if (PageHWPoison(page)) {
1419 if (WARN_ON(PageLRU(page)))
1420 isolate_lru_page(page);
1421 if (page_mapped(page))
vvv CID 1505146: Error handling issues (CHECKED_RETURN)
vvv Calling "try_to_unmap" without checking return value (as is done elsewhere 4 out of 5 times).
1422 try_to_unmap(page, TTU_IGNORE_MLOCK);
1423 continue;
1424 }
1425
1426 if (!get_page_unless_zero(page))
1427 continue;
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 <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1505146 ("Error handling issues")
Fixes: 013339df116c ("mm/rmap: always do TTU_IGNORE_ACCESS")
Thanks for your attention!
--
Coverity-bot
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Coverity: do_migrate_range(): Error handling issues
2021-06-08 18:05 Coverity: do_migrate_range(): Error handling issues coverity-bot
@ 2021-06-08 19:35 ` Linus Torvalds
2021-06-08 20:21 ` Shakeel Butt
2021-06-08 23:12 ` Kees Cook
0 siblings, 2 replies; 4+ messages in thread
From: Linus Torvalds @ 2021-06-08 19:35 UTC (permalink / raw)
To: coverity-bot
Cc: Shakeel Butt, Andrew Morton, Johannes Weiner,
Gustavo A. R. Silva, Linux Next Mailing List
On Tue, Jun 8, 2021 at 11:05 AM coverity-bot <keescook@chromium.org> wrote:
>
> You're getting this email because you were associated with the identified
> lines of code (noted below) that were touched by commits:
>
> Tue Dec 15 12:13:39 2020 -0800
> 013339df116c ("mm/rmap: always do TTU_IGNORE_ACCESS")
Heh. Entirely independent change, but whatever..
That does mean that the suggested
> Fixes: 013339df116c ("mm/rmap: always do TTU_IGNORE_ACCESS")
tag would have been entirely bogus. That commit didn't actually change
the coverity issue in question anyway, it just happened to change that
line in other ways.
Maybe the coverity-bot logic could actually try to see if a coverity
report changes, which it wouldn't have done here.
The report itself is:
> 1421 if (page_mapped(page))
> vvv CID 1505146: Error handling issues (CHECKED_RETURN)
> vvv Calling "try_to_unmap" without checking return value (as is done elsewhere 4 out of 5 times).
> 1422 try_to_unmap(page, TTU_IGNORE_MLOCK);
Hmm. That "4 out of 5 times" seems misleading. It implies that 80% of
callers check the return value.
But when I actually grep for it, I see 4 call-sites that don't check
the return value, and 5 that do. So it's more like 55% of callers that
do check.
Anyway, this is a false positive. You don't have to check the return
value. The main example of doing so is the page-out code in
mm/vmscan.c, and it does so just to keep track of success/fail
statistics.
Linus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Coverity: do_migrate_range(): Error handling issues
2021-06-08 19:35 ` Linus Torvalds
@ 2021-06-08 20:21 ` Shakeel Butt
2021-06-08 23:12 ` Kees Cook
1 sibling, 0 replies; 4+ messages in thread
From: Shakeel Butt @ 2021-06-08 20:21 UTC (permalink / raw)
To: Linus Torvalds, Yang Shi, Hugh Dickins
Cc: coverity-bot, Andrew Morton, Johannes Weiner,
Gustavo A. R. Silva, Linux Next Mailing List
On Tue, Jun 8, 2021 at 12:35 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
[...]
>
> Anyway, this is a false positive. You don't have to check the return
> value. The main example of doing so is the page-out code in
> mm/vmscan.c, and it does so just to keep track of success/fail
> statistics.
BTW there is an in-fly patch to make the try_to_umap() void function.
https://lore.kernel.org/lkml/alpine.LSU.2.11.2106031527070.11704@eggly.anvils/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Coverity: do_migrate_range(): Error handling issues
2021-06-08 19:35 ` Linus Torvalds
2021-06-08 20:21 ` Shakeel Butt
@ 2021-06-08 23:12 ` Kees Cook
1 sibling, 0 replies; 4+ messages in thread
From: Kees Cook @ 2021-06-08 23:12 UTC (permalink / raw)
To: Linus Torvalds
Cc: Shakeel Butt, Andrew Morton, Johannes Weiner,
Gustavo A. R. Silva, Linux Next Mailing List
On Tue, Jun 08, 2021 at 12:35:34PM -0700, Linus Torvalds wrote:
> But when I actually grep for it, I see 4 call-sites that don't check
> the return value, and 5 that do. So it's more like 55% of callers that
> do check.
>
> Anyway, this is a false positive. You don't have to check the return
> value. The main example of doing so is the page-out code in
> mm/vmscan.c, and it does so just to keep track of success/fail
> statistics.
Okay, cool; thanks for taking a closer look at it.
--
Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-08 23:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-08 18:05 Coverity: do_migrate_range(): Error handling issues coverity-bot
2021-06-08 19:35 ` Linus Torvalds
2021-06-08 20:21 ` Shakeel Butt
2021-06-08 23:12 ` Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.