On Thu, Jan 14, 2016 at 3:38 PM, Michal Hocko <mhocko@xxxxxxxxxx> wrote:
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?
It was pretty easy to reproduce in a stress test setup, where we spawn a process, put it in a mem cgroup and setup the threshold, have it allocate a lot of memory quickly (crossing the threshold), unregister the event, kill and repeat. Usually within 30 mins.
>
> Fixed by only freeing after synchronize_rcu().
>
Fixes: 8c7577637ca3 ("memcg: free spare array to avoid memory leak")
> Signed-off-by: Martijn Coenen <maco@xxxxxxxxxx>
Cc: stable
Acked-by: Michal Hocko <mhocko@xxxxxxxx>
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