* Coverity: shrink_node_memcgs(): Null pointer dereferences
@ 2019-11-12 1:35 coverity-bot
2019-11-12 15:21 ` Johannes Weiner
0 siblings, 1 reply; 5+ messages in thread
From: coverity-bot @ 2019-11-12 1:35 UTC (permalink / raw)
To: Johannes Weiner
Cc: Stephen Rothwell, Andrew Morton, Gustavo A. R. Silva, linux-next
Hello!
This is an experimental automated report about issues detected by Coverity
from a scan of next-20191108 as part of the linux-next weekly 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 recent commits:
c34aa3085f94 ("mm-vmscan-split-shrink_node-into-node-part-and-memcgs-part-fix")
Coverity reported the following:
*** CID 1487844: Null pointer dereferences (NULL_RETURNS)
/mm/vmscan.c: 2695 in shrink_node_memcgs()
2689 memcg = mem_cgroup_iter(target_memcg, NULL, NULL);
2690 do {
2691 struct lruvec *lruvec = mem_cgroup_lruvec(memcg, pgdat);
2692 unsigned long reclaimed;
2693 unsigned long scanned;
2694
vvv CID 1487844: Null pointer dereferences (NULL_RETURNS)
vvv Dereferencing a pointer that might be "NULL" "memcg" when calling "mem_cgroup_protected".
2695 switch (mem_cgroup_protected(target_memcg, memcg)) {
2696 case MEMCG_PROT_MIN:
2697 /*
2698 * Hard protection.
2699 * If there is no reclaimable memory, OOM.
2700 */
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: 1487844 ("Null pointer dereferences")
Fixes: c34aa3085f94 ("mm-vmscan-split-shrink_node-into-node-part-and-memcgs-part-fix")
Thanks for your attention!
--
Coverity-bot
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Coverity: shrink_node_memcgs(): Null pointer dereferences
2019-11-12 1:35 Coverity: shrink_node_memcgs(): Null pointer dereferences coverity-bot
@ 2019-11-12 15:21 ` Johannes Weiner
2019-11-12 21:04 ` Kees Cook
0 siblings, 1 reply; 5+ messages in thread
From: Johannes Weiner @ 2019-11-12 15:21 UTC (permalink / raw)
To: coverity-bot
Cc: Stephen Rothwell, Andrew Morton, Gustavo A. R. Silva, linux-next
On Mon, Nov 11, 2019 at 05:35:37PM -0800, coverity-bot wrote:
> Hello!
>
> This is an experimental automated report about issues detected by Coverity
> from a scan of next-20191108 as part of the linux-next weekly 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 recent commits:
>
> c34aa3085f94 ("mm-vmscan-split-shrink_node-into-node-part-and-memcgs-part-fix")
>
> Coverity reported the following:
>
> *** CID 1487844: Null pointer dereferences (NULL_RETURNS)
> /mm/vmscan.c: 2695 in shrink_node_memcgs()
> 2689 memcg = mem_cgroup_iter(target_memcg, NULL, NULL);
> 2690 do {
> 2691 struct lruvec *lruvec = mem_cgroup_lruvec(memcg, pgdat);
> 2692 unsigned long reclaimed;
> 2693 unsigned long scanned;
> 2694
> vvv CID 1487844: Null pointer dereferences (NULL_RETURNS)
> vvv Dereferencing a pointer that might be "NULL" "memcg" when calling "mem_cgroup_protected".
> 2695 switch (mem_cgroup_protected(target_memcg, memcg)) {
This appears to be a false alarm.
All the "culprit" patch did was rename the local variable
"target_memcg".
And while it's correct that memcg can be NULL (befor and after this
patch), it's the case only when mem_cgroup_disabled(), and
mem_cgroup_protected() checks for this case.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Coverity: shrink_node_memcgs(): Null pointer dereferences
2019-11-12 15:21 ` Johannes Weiner
@ 2019-11-12 21:04 ` Kees Cook
2019-11-12 21:56 ` Johannes Weiner
0 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2019-11-12 21:04 UTC (permalink / raw)
To: Johannes Weiner
Cc: Stephen Rothwell, Andrew Morton, Gustavo A. R. Silva, linux-next
On Tue, Nov 12, 2019 at 10:21:23AM -0500, Johannes Weiner wrote:
> On Mon, Nov 11, 2019 at 05:35:37PM -0800, coverity-bot wrote:
> > Hello!
> >
> > This is an experimental automated report about issues detected by Coverity
> > from a scan of next-20191108 as part of the linux-next weekly 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 recent commits:
> >
> > c34aa3085f94 ("mm-vmscan-split-shrink_node-into-node-part-and-memcgs-part-fix")
> >
> > Coverity reported the following:
> >
> > *** CID 1487844: Null pointer dereferences (NULL_RETURNS)
> > /mm/vmscan.c: 2695 in shrink_node_memcgs()
> > 2689 memcg = mem_cgroup_iter(target_memcg, NULL, NULL);
> > 2690 do {
> > 2691 struct lruvec *lruvec = mem_cgroup_lruvec(memcg, pgdat);
> > 2692 unsigned long reclaimed;
> > 2693 unsigned long scanned;
> > 2694
> > vvv CID 1487844: Null pointer dereferences (NULL_RETURNS)
> > vvv Dereferencing a pointer that might be "NULL" "memcg" when calling "mem_cgroup_protected".
> > 2695 switch (mem_cgroup_protected(target_memcg, memcg)) {
>
> This appears to be a false alarm.
Okay, thanks!
> All the "culprit" patch did was rename the local variable
> "target_memcg".
>
> And while it's correct that memcg can be NULL (befor and after this
> patch), it's the case only when mem_cgroup_disabled(), and
> mem_cgroup_protected() checks for this case.
Right, that's certainly the design. I wonder if in the interests of
defensively asserting requirements, it would be worth adding something
like this to mem_cgroup_protected():
if (WARN_ON_ONCE(!memcg))
return MEMCG_PROT_NONE;
?
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Coverity: shrink_node_memcgs(): Null pointer dereferences
2019-11-12 21:04 ` Kees Cook
@ 2019-11-12 21:56 ` Johannes Weiner
2019-11-12 22:18 ` Kees Cook
0 siblings, 1 reply; 5+ messages in thread
From: Johannes Weiner @ 2019-11-12 21:56 UTC (permalink / raw)
To: Kees Cook
Cc: Stephen Rothwell, Andrew Morton, Gustavo A. R. Silva, linux-next
On Tue, Nov 12, 2019 at 01:04:03PM -0800, Kees Cook wrote:
> On Tue, Nov 12, 2019 at 10:21:23AM -0500, Johannes Weiner wrote:
> > On Mon, Nov 11, 2019 at 05:35:37PM -0800, coverity-bot wrote:
> > > Hello!
> > >
> > > This is an experimental automated report about issues detected by Coverity
> > > from a scan of next-20191108 as part of the linux-next weekly 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 recent commits:
> > >
> > > c34aa3085f94 ("mm-vmscan-split-shrink_node-into-node-part-and-memcgs-part-fix")
> > >
> > > Coverity reported the following:
> > >
> > > *** CID 1487844: Null pointer dereferences (NULL_RETURNS)
> > > /mm/vmscan.c: 2695 in shrink_node_memcgs()
> > > 2689 memcg = mem_cgroup_iter(target_memcg, NULL, NULL);
> > > 2690 do {
> > > 2691 struct lruvec *lruvec = mem_cgroup_lruvec(memcg, pgdat);
> > > 2692 unsigned long reclaimed;
> > > 2693 unsigned long scanned;
> > > 2694
> > > vvv CID 1487844: Null pointer dereferences (NULL_RETURNS)
> > > vvv Dereferencing a pointer that might be "NULL" "memcg" when calling "mem_cgroup_protected".
> > > 2695 switch (mem_cgroup_protected(target_memcg, memcg)) {
> >
> > This appears to be a false alarm.
>
> Okay, thanks!
>
> > All the "culprit" patch did was rename the local variable
> > "target_memcg".
> >
> > And while it's correct that memcg can be NULL (befor and after this
> > patch), it's the case only when mem_cgroup_disabled(), and
> > mem_cgroup_protected() checks for this case.
>
> Right, that's certainly the design. I wonder if in the interests of
> defensively asserting requirements, it would be worth adding something
> like this to mem_cgroup_protected():
>
> if (WARN_ON_ONCE(!memcg))
> return MEMCG_PROT_NONE;
I'm having trouble enumerating the number of places where we would
crash in reclaim if memcg were zero while the mem controller is on.
And even if we annotated all of them and dreamed up more or less
sensical exit values for all of these places, we'd quickly panic due
to failing page reclaim.
There is no graceful exit strategy here. We may as well take the crash
right away, without having to clutter up the code.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Coverity: shrink_node_memcgs(): Null pointer dereferences
2019-11-12 21:56 ` Johannes Weiner
@ 2019-11-12 22:18 ` Kees Cook
0 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2019-11-12 22:18 UTC (permalink / raw)
To: Johannes Weiner
Cc: Stephen Rothwell, Andrew Morton, Gustavo A. R. Silva, linux-next
On Tue, Nov 12, 2019 at 04:56:52PM -0500, Johannes Weiner wrote:
> On Tue, Nov 12, 2019 at 01:04:03PM -0800, Kees Cook wrote:
> > On Tue, Nov 12, 2019 at 10:21:23AM -0500, Johannes Weiner wrote:
> > > On Mon, Nov 11, 2019 at 05:35:37PM -0800, coverity-bot wrote:
> > > > Hello!
> > > >
> > > > This is an experimental automated report about issues detected by Coverity
> > > > from a scan of next-20191108 as part of the linux-next weekly 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 recent commits:
> > > >
> > > > c34aa3085f94 ("mm-vmscan-split-shrink_node-into-node-part-and-memcgs-part-fix")
> > > >
> > > > Coverity reported the following:
> > > >
> > > > *** CID 1487844: Null pointer dereferences (NULL_RETURNS)
> > > > /mm/vmscan.c: 2695 in shrink_node_memcgs()
> > > > 2689 memcg = mem_cgroup_iter(target_memcg, NULL, NULL);
> > > > 2690 do {
> > > > 2691 struct lruvec *lruvec = mem_cgroup_lruvec(memcg, pgdat);
> > > > 2692 unsigned long reclaimed;
> > > > 2693 unsigned long scanned;
> > > > 2694
> > > > vvv CID 1487844: Null pointer dereferences (NULL_RETURNS)
> > > > vvv Dereferencing a pointer that might be "NULL" "memcg" when calling "mem_cgroup_protected".
> > > > 2695 switch (mem_cgroup_protected(target_memcg, memcg)) {
> > >
> > > This appears to be a false alarm.
> >
> > Okay, thanks!
> >
> > > All the "culprit" patch did was rename the local variable
> > > "target_memcg".
> > >
> > > And while it's correct that memcg can be NULL (befor and after this
> > > patch), it's the case only when mem_cgroup_disabled(), and
> > > mem_cgroup_protected() checks for this case.
> >
> > Right, that's certainly the design. I wonder if in the interests of
> > defensively asserting requirements, it would be worth adding something
> > like this to mem_cgroup_protected():
> >
> > if (WARN_ON_ONCE(!memcg))
> > return MEMCG_PROT_NONE;
>
> I'm having trouble enumerating the number of places where we would
> crash in reclaim if memcg were zero while the mem controller is on.
>
> And even if we annotated all of them and dreamed up more or less
> sensical exit values for all of these places, we'd quickly panic due
> to failing page reclaim.
>
> There is no graceful exit strategy here. We may as well take the crash
> right away, without having to clutter up the code.
Okay, cool. I was just thinking mem_cgroup_protected() would be central
enough since it's already tested in tons of places.
Thanks for looking at it!
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-11-12 22:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-12 1:35 Coverity: shrink_node_memcgs(): Null pointer dereferences coverity-bot
2019-11-12 15:21 ` Johannes Weiner
2019-11-12 21:04 ` Kees Cook
2019-11-12 21:56 ` Johannes Weiner
2019-11-12 22:18 ` Kees Cook
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).