Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753274Ab2FKJSc (ORCPT ); Mon, 11 Jun 2012 05:18:32 -0400 Received: from mail-gh0-f174.google.com ([209.85.160.174]:52355 "EHLO mail-gh0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752719Ab2FKJS1 (ORCPT ); Mon, 11 Jun 2012 05:18:27 -0400 From: kosaki.motohiro@gmail.com To: linux-kernel@vger.kernel.org Cc: linux-mm@kvack.org, Andrew Morton , Dave Jones , Mel Gorman , Christoph Lameter , stable@vger.kernel.org, KOSAKI Motohiro , Andrew Morton Subject: [PATCH 3/6] mempolicy: fix a race in shared_policy_replace() Date: Mon, 11 Jun 2012 05:17:27 -0400 Message-Id: <1339406250-10169-4-git-send-email-kosaki.motohiro@gmail.com> X-Mailer: git-send-email 1.7.1 In-Reply-To: <1339406250-10169-1-git-send-email-kosaki.motohiro@gmail.com> References: <1339406250-10169-1-git-send-email-kosaki.motohiro@gmail.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3338 Lines: 125 From: KOSAKI Motohiro shared_policy_replace() uses sp_alloc() wrongly. 1) sp_node can't be dereferenced when not holding sp->lock and 2) another thread can modify sp_node when sp->lock is unlocked. This patch fixes them. Note: The bug was introduced pre-git age (IOW, before 2.6.12-rc2). I believe nobody uses this feature in production systems. Cc: Dave Jones , Cc: Mel Gorman Cc: Christoph Lameter , Cc: Andrew Morton Cc: Signed-off-by: KOSAKI Motohiro --- mm/mempolicy.c | 55 ++++++++++++++++++++++++++++++++++++++----------------- 1 files changed, 38 insertions(+), 17 deletions(-) diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 9505cb9..d97d2db 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -2158,6 +2158,14 @@ static void sp_delete(struct shared_policy *sp, struct sp_node *n) kmem_cache_free(sn_cache, n); } +static void sp_node_init(struct sp_node *node, unsigned long start, + unsigned long end, struct mempolicy *pol) +{ + node->start = start; + node->end = end; + node->policy = pol; +} + static struct sp_node *sp_alloc(unsigned long start, unsigned long end, struct mempolicy *pol) { @@ -2174,10 +2182,7 @@ static struct sp_node *sp_alloc(unsigned long start, unsigned long end, return NULL; } newpol->flags |= MPOL_F_SHARED; - - n->start = start; - n->end = end; - n->policy = newpol; + sp_node_init(n, start, end, newpol); return n; } @@ -2186,7 +2191,9 @@ static struct sp_node *sp_alloc(unsigned long start, unsigned long end, static int shared_policy_replace(struct shared_policy *sp, unsigned long start, unsigned long end, struct sp_node *new) { + int err; struct sp_node *n, *new2 = NULL; + struct mempolicy *new2_pol = NULL; restart: spin_lock(&sp->lock); @@ -2202,16 +2209,16 @@ restart: } else { /* Old policy spanning whole new range. */ if (n->end > end) { - if (!new2) { - spin_unlock(&sp->lock); - new2 = sp_alloc(end, n->end, n->policy); - if (!new2) - return -ENOMEM; - goto restart; - } - n->end = start; + if (!new2) + goto alloc_new2; + + *new2_pol = *n->policy; + atomic_set(&new2_pol->refcnt, 1); + sp_node_init(new2, n->end, end, new2_pol); sp_insert(sp, new2); + n->end = start; new2 = NULL; + new2_pol = NULL; break; } else n->end = start; @@ -2223,11 +2230,25 @@ restart: if (new) sp_insert(sp, new); spin_unlock(&sp->lock); - if (new2) { - mpol_put(new2->policy); - kmem_cache_free(sn_cache, new2); - } - return 0; + err = 0; + + err_out: + if (new2_pol) + mpol_put(new2_pol); + if (new2) + kmem_cache_free(sn_cache, new2); + return err; + + alloc_new2: + spin_unlock(&sp->lock); + err = -ENOMEM; + new2 = kmem_cache_alloc(sn_cache, GFP_KERNEL); + if (!new2) + goto err_out; + new2_pol = kmem_cache_alloc(policy_cache, GFP_KERNEL); + if (!new2_pol) + goto err_out; + goto restart; } /** -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/