[rh7] mm/memcg: fix css_tryget(), css_put() imbalance

Submitted by Andrey Ryabinin on July 15, 2020, 1:25 p.m.

Details

Message ID 20200715132557.2249-1-aryabinin@virtuozzo.com
State New
Series "mm/memcg: fix css_tryget(), css_put() imbalance"
Headers show

Commit Message

Andrey Ryabinin July 15, 2020, 1:25 p.m.
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(-)

Patch hide | download patch | download mbox

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);

Comments

Konstantin Khorenko July 15, 2020, 3:11 p.m.
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);
>