All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] memcg: fix per_node_info cleanup
@ 2018-04-06 10:09 ` Michal Hocko
  0 siblings, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2018-04-06 10:09 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Johannes Weiner, syzbot+8a5de3cce7cdc70e9ebe, Andrey Ryabinin,
	LKML, linux-mm, Michal Hocko

From: Michal Hocko <mhocko@suse.com>

syzbot has triggered a NULL ptr dereference when allocation fault
injection enforces a failure and alloc_mem_cgroup_per_node_info
initializes memcg->nodeinfo only half way through. __mem_cgroup_free
still tries to free all per-node data and dereferences pn->lruvec_stat_cpu
unconditioanlly even if the specific per-node data hasn't been
initialized.

The bug is quite unlikely to hit because small allocations do not fail
and we would need quite some numa nodes to make struct mem_cgroup_per_node
large enough to cross the costly order.

Reported-by: syzbot+8a5de3cce7cdc70e9ebe@syzkaller.appspotmail.com
Fixes: 00f3ca2c2d66 ("mm: memcontrol: per-lruvec stats infrastructure")
Reviewed-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---

Hi!
Previously posted [1] based on the syzkaller report [2]. I haven't heard
back from syzkaller but this seems like the right fix. Andrew could you
add this to the pile?

[1] http://lkml.kernel.org/r/20180403105048.GK5501@dhcp22.suse.cz
[2] http://lkml.kernel.org/r/001a113fe4c0a623b10568bb75ea@google.com

 mm/memcontrol.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 7667ea9daf4f..8c2ed1c2b72c 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4340,6 +4340,9 @@ static void free_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
 {
 	struct mem_cgroup_per_node *pn = memcg->nodeinfo[node];
 
+	if (!pn)
+		return;
+
 	free_percpu(pn->lruvec_stat_cpu);
 	kfree(pn);
 }
-- 
2.16.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] memcg: fix per_node_info cleanup
@ 2018-04-06 10:09 ` Michal Hocko
  0 siblings, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2018-04-06 10:09 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Johannes Weiner, syzbot+8a5de3cce7cdc70e9ebe, Andrey Ryabinin,
	LKML, linux-mm, Michal Hocko

From: Michal Hocko <mhocko@suse.com>

syzbot has triggered a NULL ptr dereference when allocation fault
injection enforces a failure and alloc_mem_cgroup_per_node_info
initializes memcg->nodeinfo only half way through. __mem_cgroup_free
still tries to free all per-node data and dereferences pn->lruvec_stat_cpu
unconditioanlly even if the specific per-node data hasn't been
initialized.

The bug is quite unlikely to hit because small allocations do not fail
and we would need quite some numa nodes to make struct mem_cgroup_per_node
large enough to cross the costly order.

Reported-by: syzbot+8a5de3cce7cdc70e9ebe@syzkaller.appspotmail.com
Fixes: 00f3ca2c2d66 ("mm: memcontrol: per-lruvec stats infrastructure")
Reviewed-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---

Hi!
Previously posted [1] based on the syzkaller report [2]. I haven't heard
back from syzkaller but this seems like the right fix. Andrew could you
add this to the pile?

[1] http://lkml.kernel.org/r/20180403105048.GK5501@dhcp22.suse.cz
[2] http://lkml.kernel.org/r/001a113fe4c0a623b10568bb75ea@google.com

 mm/memcontrol.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 7667ea9daf4f..8c2ed1c2b72c 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4340,6 +4340,9 @@ static void free_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
 {
 	struct mem_cgroup_per_node *pn = memcg->nodeinfo[node];
 
+	if (!pn)
+		return;
+
 	free_percpu(pn->lruvec_stat_cpu);
 	kfree(pn);
 }
-- 
2.16.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] memcg: fix per_node_info cleanup
  2018-04-06 10:09 ` Michal Hocko
  (?)
@ 2018-04-10 13:50 ` Sasha Levin
  2018-04-10 14:01   ` Michal Hocko
  -1 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2018-04-10 13:50 UTC (permalink / raw)
  To: Sasha Levin, Michal Hocko, Michal Hocko, Andrew Morton
  Cc: Johannes Weiner, stable

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 00f3ca2c2d66 mm: memcontrol: per-lruvec stats infrastructure.

The bot has also determined it's probably a bug fixing patch. (score: 40.3266)

The bot has tested the following trees: v4.16.1, v4.15.16, v4.14.33.

v4.16.1: Build OK!
v4.15.16: Failed to apply! Possible dependencies:
    a983b5ebee57 ("mm: memcontrol: fix excessive complexity in memory.stat reporting")

v4.14.33: Failed to apply! Possible dependencies:
    a983b5ebee57 ("mm: memcontrol: fix excessive complexity in memory.stat reporting")


--
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] memcg: fix per_node_info cleanup
  2018-04-10 13:50 ` Sasha Levin
@ 2018-04-10 14:01   ` Michal Hocko
  2018-04-10 17:55     ` Sasha Levin
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2018-04-10 14:01 UTC (permalink / raw)
  To: Sasha Levin; +Cc: Andrew Morton, Johannes Weiner, stable

On Tue 10-04-18 13:50:30, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: 00f3ca2c2d66 mm: memcontrol: per-lruvec stats infrastructure.

Yes and the changelog states conditions which are quite hard to achieve
to have this patch in the stable tree. So I am not really convinced it
is worth backporting even though the patch itself is trivial.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] memcg: fix per_node_info cleanup
  2018-04-10 14:01   ` Michal Hocko
@ 2018-04-10 17:55     ` Sasha Levin
  2018-04-10 20:37       ` Michal Hocko
  0 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2018-04-10 17:55 UTC (permalink / raw)
  To: Michal Hocko; +Cc: Andrew Morton, Johannes Weiner, stable

On Tue, Apr 10, 2018 at 04:01:32PM +0200, Michal Hocko wrote:
>On Tue 10-04-18 13:50:30, Sasha Levin wrote:
>> Hi,
>>
>> [This is an automated email]
>>
>> This commit has been processed because it contains a "Fixes:" tag,
>> fixing commit: 00f3ca2c2d66 mm: memcontrol: per-lruvec stats infrastructure.
>
>Yes and the changelog states conditions which are quite hard to achieve
>to have this patch in the stable tree. So I am not really convinced it
>is worth backporting even though the patch itself is trivial.

Quite hard to achieve, but the changelog also indicated that this was
actually hit and is not just theoretical. Is there something that will
prevent this from happening on "real" workloads?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] memcg: fix per_node_info cleanup
  2018-04-10 17:55     ` Sasha Levin
@ 2018-04-10 20:37       ` Michal Hocko
  0 siblings, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2018-04-10 20:37 UTC (permalink / raw)
  To: Sasha Levin; +Cc: Andrew Morton, Johannes Weiner, stable

On Tue 10-04-18 17:55:13, Sasha Levin wrote:
> On Tue, Apr 10, 2018 at 04:01:32PM +0200, Michal Hocko wrote:
> >On Tue 10-04-18 13:50:30, Sasha Levin wrote:
> >> Hi,
> >>
> >> [This is an automated email]
> >>
> >> This commit has been processed because it contains a "Fixes:" tag,
> >> fixing commit: 00f3ca2c2d66 mm: memcontrol: per-lruvec stats infrastructure.
> >
> >Yes and the changelog states conditions which are quite hard to achieve
> >to have this patch in the stable tree. So I am not really convinced it
> >is worth backporting even though the patch itself is trivial.
> 
> Quite hard to achieve, but the changelog also indicated that this was
> actually hit and is not just theoretical. Is there something that will
> prevent this from happening on "real" workloads?

Well, small allocations do not really fail and it would take to
configure a really large NUMA system to achieve the same. So I am
skeptical.

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-04-10 20:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-06 10:09 [PATCH] memcg: fix per_node_info cleanup Michal Hocko
2018-04-06 10:09 ` Michal Hocko
2018-04-10 13:50 ` Sasha Levin
2018-04-10 14:01   ` Michal Hocko
2018-04-10 17:55     ` Sasha Levin
2018-04-10 20:37       ` Michal Hocko

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.