| Message ID | 20200715132557.2249-1-aryabinin@virtuozzo.com |
|---|---|
| State | New |
| Series | "mm/memcg: fix css_tryget(), css_put() imbalance" |
| Headers | show |
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 34c9c4745594..d989111f8d18 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1520,7 +1520,7 @@ mem_cgroup_iter_load(struct mem_cgroup_reclaim_iter *iter, struct mem_cgroup *root, int *sequence) { - struct mem_cgroup *position = NULL; + struct mem_cgroup *position; unsigned seq; /* @@ -1533,6 +1533,7 @@ mem_cgroup_iter_load(struct mem_cgroup_reclaim_iter *iter, */ *sequence = atomic_read(&root->dead_count); retry: + position = NULL; seq = read_seqbegin(&iter->last_visited_lock); if (iter->last_dead_count == *sequence) { position = READ_ONCE(iter->last_visited);
On 07/15/2020 04:25 PM, Andrey Ryabinin wrote: > If mem_cgroup_iter_load() goes to retry after failed read_seqretry(): > > retry: > seq = read_seqbegin(&iter->last_visited_lock); > if (iter->last_dead_count == *sequence) { > position = READ_ONCE(iter->last_visited); > > if (read_seqretry(&iter->last_visited_lock, seq)) > goto retry: > > and the condition is (iter->last_dead_count == *sequence) false, > mem_cgroup_iter_load() will return non-NULL position, without doing > css_tryget(). This leads to extra css_put() in mem_cgroup_iter_update() > and kernel crash later. > > Fix this by NULLifying 'position' on each retry. > > https://jira.sw.ru/browse/PSBM-98148 > Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com> Acked-by: Konstantin Khorenko <khorenko@virtuozzo.com> > --- > mm/memcontrol.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 34c9c4745594..d989111f8d18 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1520,7 +1520,7 @@ mem_cgroup_iter_load(struct mem_cgroup_reclaim_iter *iter, > struct mem_cgroup *root, > int *sequence) > { > - struct mem_cgroup *position = NULL; > + struct mem_cgroup *position; > unsigned seq; > > /* > @@ -1533,6 +1533,7 @@ mem_cgroup_iter_load(struct mem_cgroup_reclaim_iter *iter, > */ > *sequence = atomic_read(&root->dead_count); > retry: > + position = NULL; > seq = read_seqbegin(&iter->last_visited_lock); > if (iter->last_dead_count == *sequence) { > position = READ_ONCE(iter->last_visited); >
If mem_cgroup_iter_load() goes to retry after failed read_seqretry(): retry: seq = read_seqbegin(&iter->last_visited_lock); if (iter->last_dead_count == *sequence) { position = READ_ONCE(iter->last_visited); if (read_seqretry(&iter->last_visited_lock, seq)) goto retry: and the condition is (iter->last_dead_count == *sequence) false, mem_cgroup_iter_load() will return non-NULL position, without doing css_tryget(). This leads to extra css_put() in mem_cgroup_iter_update() and kernel crash later. Fix this by NULLifying 'position' on each retry. https://jira.sw.ru/browse/PSBM-98148 Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com> --- mm/memcontrol.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)