From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759276Ab2J2P0q (ORCPT ); Mon, 29 Oct 2012 11:26:46 -0400 Received: from mail-oa0-f46.google.com ([209.85.219.46]:38794 "EHLO mail-oa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751612Ab2J2P0o (ORCPT ); Mon, 29 Oct 2012 11:26:44 -0400 MIME-Version: 1.0 In-Reply-To: <1350656442-1523-14-git-send-email-glommer@parallels.com> References: <1350656442-1523-1-git-send-email-glommer@parallels.com> <1350656442-1523-14-git-send-email-glommer@parallels.com> Date: Tue, 30 Oct 2012 00:26:43 +0900 Message-ID: Subject: Re: [PATCH v5 13/18] memcg/sl[au]b Track all the memcg children of a kmem_cache. From: JoonSoo Kim To: Glauber Costa Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, Mel Gorman , Tejun Heo , Andrew Morton , Michal Hocko , Johannes Weiner , kamezawa.hiroyu@jp.fujitsu.com, Christoph Lameter , David Rientjes , Pekka Enberg , devel@openvz.org, Suleiman Souhlal , Pekka Enberg Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2012/10/19 Glauber Costa : > +void kmem_cache_destroy_memcg_children(struct kmem_cache *s) > +{ > + struct kmem_cache *c; > + int i; > + > + if (!s->memcg_params) > + return; > + if (!s->memcg_params->is_root_cache) > + return; > + > + /* > + * If the cache is being destroyed, we trust that there is no one else > + * requesting objects from it. Even if there are, the sanity checks in > + * kmem_cache_destroy should caught this ill-case. > + * > + * Still, we don't want anyone else freeing memcg_caches under our > + * noses, which can happen if a new memcg comes to life. As usual, > + * we'll take the set_limit_mutex to protect ourselves against this. > + */ > + mutex_lock(&set_limit_mutex); > + for (i = 0; i < memcg_limited_groups_array_size; i++) { > + c = s->memcg_params->memcg_caches[i]; > + if (c) > + kmem_cache_destroy(c); > + } > + mutex_unlock(&set_limit_mutex); > +} It may cause NULL deref. Look at the following scenario. 1. some memcg slab caches has remained object. 2. start to destroy memcg. 3. schedule_delayed_work(kmem_cache_destroy_work_func, @delay 60hz) 4. all remained object is freed. 5. start to destroy root cache. 6. kmem_cache_destroy makes 's->memcg_params->memcg_caches[i]" NULL!! 7. Start delayed work function. 8. cachep in kmem_cache_destroy_work_func() may be NULL Thanks. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx120.postini.com [74.125.245.120]) by kanga.kvack.org (Postfix) with SMTP id 334456B0071 for ; Mon, 29 Oct 2012 11:26:44 -0400 (EDT) Received: by mail-oa0-f41.google.com with SMTP id k14so6085312oag.14 for ; Mon, 29 Oct 2012 08:26:43 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1350656442-1523-14-git-send-email-glommer@parallels.com> References: <1350656442-1523-1-git-send-email-glommer@parallels.com> <1350656442-1523-14-git-send-email-glommer@parallels.com> Date: Tue, 30 Oct 2012 00:26:43 +0900 Message-ID: Subject: Re: [PATCH v5 13/18] memcg/sl[au]b Track all the memcg children of a kmem_cache. From: JoonSoo Kim Content-Type: text/plain; charset=ISO-8859-1 Sender: owner-linux-mm@kvack.org List-ID: To: Glauber Costa Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, Mel Gorman , Tejun Heo , Andrew Morton , Michal Hocko , Johannes Weiner , kamezawa.hiroyu@jp.fujitsu.com, Christoph Lameter , David Rientjes , Pekka Enberg , devel@openvz.org, Suleiman Souhlal , Pekka Enberg 2012/10/19 Glauber Costa : > +void kmem_cache_destroy_memcg_children(struct kmem_cache *s) > +{ > + struct kmem_cache *c; > + int i; > + > + if (!s->memcg_params) > + return; > + if (!s->memcg_params->is_root_cache) > + return; > + > + /* > + * If the cache is being destroyed, we trust that there is no one else > + * requesting objects from it. Even if there are, the sanity checks in > + * kmem_cache_destroy should caught this ill-case. > + * > + * Still, we don't want anyone else freeing memcg_caches under our > + * noses, which can happen if a new memcg comes to life. As usual, > + * we'll take the set_limit_mutex to protect ourselves against this. > + */ > + mutex_lock(&set_limit_mutex); > + for (i = 0; i < memcg_limited_groups_array_size; i++) { > + c = s->memcg_params->memcg_caches[i]; > + if (c) > + kmem_cache_destroy(c); > + } > + mutex_unlock(&set_limit_mutex); > +} It may cause NULL deref. Look at the following scenario. 1. some memcg slab caches has remained object. 2. start to destroy memcg. 3. schedule_delayed_work(kmem_cache_destroy_work_func, @delay 60hz) 4. all remained object is freed. 5. start to destroy root cache. 6. kmem_cache_destroy makes 's->memcg_params->memcg_caches[i]" NULL!! 7. Start delayed work function. 8. cachep in kmem_cache_destroy_work_func() may be NULL Thanks. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: JoonSoo Kim Subject: Re: [PATCH v5 13/18] memcg/sl[au]b Track all the memcg children of a kmem_cache. Date: Tue, 30 Oct 2012 00:26:43 +0900 Message-ID: References: <1350656442-1523-1-git-send-email-glommer@parallels.com> <1350656442-1523-14-git-send-email-glommer@parallels.com> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=8GDGNSXKB1rpFA3MisBzUIUZn4J0HMHSpLWj22xdsJI=; b=RdgzlH0xWRsE4jCjhKAtO05mWCWQOPafslhvkTqR01XTRSqjkxGnsfQn2Bvgo3voB5 T46jkaYJGs0yUfXGQV2J3jsOzk9tccw39+BIgrWB2U65KZhKDYDJnbCmKm1w+CazD7oU w/QG4nra9Fy78Z17noWqynzrAaLLHxoAz7LFcoI+3ql544oi0B3oWs1/VWTMwBhPbI7y ytwoF9nOJAczM/9YWICurnP03bdM8FGl4m6aqn9ka/kQuIgYJgdDNsJj1OzVOAPmOM2/ c5cABZtAK5UgRmhOjt/nGYcTnWifzqpdN9vyUHo1AqACvGH97Jy+ptLfLjfZP3zw+pVd veeA== In-Reply-To: <1350656442-1523-14-git-send-email-glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> Sender: cgroups-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Glauber Costa Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mel Gorman , Tejun Heo , Andrew Morton , Michal Hocko , Johannes Weiner , kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org, Christoph Lameter , David Rientjes , Pekka Enberg , devel-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org, Suleiman Souhlal , Pekka Enberg 2012/10/19 Glauber Costa : > +void kmem_cache_destroy_memcg_children(struct kmem_cache *s) > +{ > + struct kmem_cache *c; > + int i; > + > + if (!s->memcg_params) > + return; > + if (!s->memcg_params->is_root_cache) > + return; > + > + /* > + * If the cache is being destroyed, we trust that there is no one else > + * requesting objects from it. Even if there are, the sanity checks in > + * kmem_cache_destroy should caught this ill-case. > + * > + * Still, we don't want anyone else freeing memcg_caches under our > + * noses, which can happen if a new memcg comes to life. As usual, > + * we'll take the set_limit_mutex to protect ourselves against this. > + */ > + mutex_lock(&set_limit_mutex); > + for (i = 0; i < memcg_limited_groups_array_size; i++) { > + c = s->memcg_params->memcg_caches[i]; > + if (c) > + kmem_cache_destroy(c); > + } > + mutex_unlock(&set_limit_mutex); > +} It may cause NULL deref. Look at the following scenario. 1. some memcg slab caches has remained object. 2. start to destroy memcg. 3. schedule_delayed_work(kmem_cache_destroy_work_func, @delay 60hz) 4. all remained object is freed. 5. start to destroy root cache. 6. kmem_cache_destroy makes 's->memcg_params->memcg_caches[i]" NULL!! 7. Start delayed work function. 8. cachep in kmem_cache_destroy_work_func() may be NULL Thanks.