Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1309335

Re: [PATCH] memcg: Only free spare array when readers are done

From Michal Hocko <mhocko@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH] memcg: Only free spare array when readers are done
Date 2016-01-14 15:40 +0100
Message-ID <qQRea-8oq-29@gated-at.bofh.it> (permalink)
References <qQQrM-7NG-29@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu 14-01-16 14:33:52, Martijn Coenen wrote:
> A spare array holding mem cgroup threshold events is kept around
> to make sure we can always safely deregister an event and have an
> array to store the new set of events in.
> 
> In the scenario where we're going from 1 to 0 registered events, the
> pointer to the primary array containing 1 event is copied to the spare
> slot, and then the spare slot is freed because no events are left.
> However, it is freed before calling synchronize_rcu(), which means
> readers may still be accessing threshold->primary after it is freed.

Have you seen this triggering in the real life?

> 
> Fixed by only freeing after synchronize_rcu().
> 

Fixes: 8c7577637ca3 ("memcg: free spare array to avoid memory leak")
> Signed-off-by: Martijn Coenen <maco@google.com>
Cc: stable

Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!

> ---
>  mm/memcontrol.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 14cb1db..73228b6 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3522,16 +3522,17 @@ static void
> __mem_cgroup_usage_unregister_event(struct mem_cgroup *memcg,
>  swap_buffers:
>  	/* Swap primary and spare array */
>  	thresholds->spare = thresholds->primary;
> -	/* If all events are unregistered, free the spare array */
> -	if (!new) {
> -		kfree(thresholds->spare);
> -		thresholds->spare = NULL;
> -	}
> 
>  	rcu_assign_pointer(thresholds->primary, new);
> 
>  	/* To be sure that nobody uses thresholds */
>  	synchronize_rcu();
> +
> +	/* If all events are unregistered, free the spare array */
> +	if (!new) {
> +		kfree(thresholds->spare);
> +		thresholds->spare = NULL;
> +	}
>  unlock:
>  	mutex_unlock(&memcg->thresholds_lock);
>  }
> -- 
> 2.6.0.rc2.230.g3dd15c0

-- 
Michal Hocko
SUSE Labs

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] memcg: Only free spare array when readers are done Martijn Coenen <maco@google.com> - 2016-01-14 14:50 +0100
  Re: [PATCH] memcg: Only free spare array when readers are done Michal Hocko <mhocko@kernel.org> - 2016-01-14 15:40 +0100
    Re: [PATCH] memcg: Only free spare array when readers are done Martijn Coenen <maco@google.com> - 2016-01-14 16:40 +0100

csiph-web