Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755831Ab0H3QAg (ORCPT ); Mon, 30 Aug 2010 12:00:36 -0400 Received: from hera.kernel.org ([140.211.167.34]:39667 "EHLO hera.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754893Ab0H3QAf (ORCPT ); Mon, 30 Aug 2010 12:00:35 -0400 Message-ID: <4C7BD595.6080102@kernel.org> Date: Mon, 30 Aug 2010 18:00:21 +0200 From: Tejun Heo User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.8) Gecko/20100802 Thunderbird/3.1.2 MIME-Version: 1.0 To: Jeffrey Carlyle CC: OLUSANYA SOYANNWO , linux-kernel@vger.kernel.org, Hu Tao , jaxboe@fusionio.com, torvalds@osdl.org Subject: Re: [PATCH v2] scatterlist: prevent invalid free when alloc fails References: <4C784B04.7060604@kernel.org> <20100830154825.E4E2560255F@il93ubuntu.localdomain> In-Reply-To: <20100830154825.E4E2560255F@il93ubuntu.localdomain> X-Enigmail-Version: 1.1.1 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.2.3 (hera.kernel.org [127.0.0.1]); Mon, 30 Aug 2010 16:00:26 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1750 Lines: 53 Hello, On 08/30/2010 05:01 PM, Jeffrey Carlyle wrote: > When alloc fails, free_table is being called. Depending on the number of > bytes requested, we determine if we are going to call _get_free_page() > or kmalloc(). When alloc fails, our math is wrong (due to sg_size - 1), > and the last buffer is wrongfully assumed to have been allocated by > kmalloc. Hence, kfree gets called and a panic occurs. > > Signed-off-by: Jeffrey Carlyle > Signed-off-by: Olusanya Soyannwo > Cc: Tejun Heo > Cc: Jens Axboe Supposing it's verified to fix the same issue, Acked-by: Tejun Heo trivial suggestions below, > --- > lib/scatterlist.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/lib/scatterlist.c b/lib/scatterlist.c > index a5ec428..9bc637f 100644 > --- a/lib/scatterlist.c > +++ b/lib/scatterlist.c > @@ -248,8 +248,14 @@ int __sg_alloc_table(struct sg_table *table, unsigned int nents, > left -= sg_size; > > sg = alloc_fn(alloc_size, gfp_mask); > - if (unlikely(!sg)) > + if (unlikely(!sg)) { > + /* > + * Adjust entry count so that proper free function is > + * used in sg_kfree. > + */ I think it would be better why the adjustment is necessary. IOW, something like "Adjust entry count to reflect that the last entry of the previous table won't be used for linkage. Without this, sg_kfree() may get confused." Thanks. -- tejun -- 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/