From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161104Ab2COQ5y (ORCPT ); Thu, 15 Mar 2012 12:57:54 -0400 Received: from mail-pz0-f46.google.com ([209.85.210.46]:44591 "EHLO mail-pz0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161055Ab2COQ5s (ORCPT ); Thu, 15 Mar 2012 12:57:48 -0400 Message-ID: <4F621F87.3040408@gmail.com> Date: Thu, 15 Mar 2012 09:57:43 -0700 From: Dirk Brandewie User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.1) Gecko/20120216 Thunderbird/10.0.1 MIME-Version: 1.0 To: Dan Carpenter CC: Grant Likely , Dirk Brandewie , Rob Herring , Anton Vorontsov , MyungJoo Ham , Kyungmin Park , Philip Rakity , linux-kernel@vger.kernel.org, devicetree-discuss@lists.ozlabs.org, kernel-janitors@vger.kernel.org Subject: Re: [patch] max17042_battery: fix a couple buffer overflows References: <20120315113732.GA364@elgon.mountain> In-Reply-To: <20120315113732.GA364@elgon.mountain> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/15/2012 04:37 AM, Dan Carpenter wrote: > There are a couple issues here caused by confusion between sizeof() > and ARRAY_SIZE(). "table_size" should be the number of elements, but we > should allocate it with kcalloc() so that we allocate the correct number > of bytes. > > In max17042_init_model() we don't allocate enough space so we go past > the end of the array in max17042_read_model_data() and > max17042_model_data_compare(). > > In max17042_verify_model_lock() we allocate the right amount of space > but we call max17042_read_model_data() with the wrong number of elements > and also in the for loop we go past the end of the array. > > Signed-off-by: Dan Carpenter Acked-by: Dirk Brandewie > --- > I don't have this hardware. The original code is clearly buggy, but I > can't test that my fix is correct. Please review carefully. > > diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c > index e36763a..f8cd48c 100644 > --- a/drivers/power/max17042_battery.c > +++ b/drivers/power/max17042_battery.c > @@ -328,11 +328,10 @@ static inline int max17042_model_data_compare(struct max17042_chip *chip, > static int max17042_init_model(struct max17042_chip *chip) > { > int ret; > - int table_size = > - sizeof(chip->pdata->config_data->cell_char_tbl)/sizeof(u16); > + int table_size = ARRAY_SIZE(chip->pdata->config_data->cell_char_tbl); > u16 *temp_data; > > - temp_data = kzalloc(table_size, GFP_KERNEL); > + temp_data = kcalloc(table_size, sizeof(*temp_data), GFP_KERNEL); > if (!temp_data) > return -ENOMEM; > > @@ -357,12 +356,11 @@ static int max17042_init_model(struct max17042_chip *chip) > static int max17042_verify_model_lock(struct max17042_chip *chip) > { > int i; > - int table_size = > - sizeof(chip->pdata->config_data->cell_char_tbl); > + int table_size = ARRAY_SIZE(chip->pdata->config_data->cell_char_tbl); > u16 *temp_data; > int ret = 0; > > - temp_data = kzalloc(table_size, GFP_KERNEL); > + temp_data = kcalloc(table_size, sizeof(*temp_data), GFP_KERNEL); > if (!temp_data) > return -ENOMEM; >