All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Grant Likely <grant.likely@secretlab.ca>,
	Dirk Brandewie <dirk.brandewie@gmail.com>
Cc: Rob Herring <rob.herring@calxeda.com>,
	Anton Vorontsov <anton.vorontsov@linaro.org>,
	MyungJoo Ham <myungjoo.ham@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Philip Rakity <prakity@marvell.com>,
	linux-kernel@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org,
	kernel-janitors@vger.kernel.org
Subject: [patch] max17042_battery: fix a couple buffer overflows
Date: Thu, 15 Mar 2012 11:37:32 +0000	[thread overview]
Message-ID: <20120315113732.GA364@elgon.mountain> (raw)

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 <dan.carpenter@oracle.com>
---
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;
 

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Grant Likely <grant.likely@secretlab.ca>,
	Dirk Brandewie <dirk.brandewie@gmail.com>
Cc: Rob Herring <rob.herring@calxeda.com>,
	Anton Vorontsov <anton.vorontsov@linaro.org>,
	MyungJoo Ham <myungjoo.ham@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Philip Rakity <prakity@marvell.com>,
	linux-kernel@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org,
	kernel-janitors@vger.kernel.org
Subject: [patch] max17042_battery: fix a couple buffer overflows
Date: Thu, 15 Mar 2012 14:37:32 +0300	[thread overview]
Message-ID: <20120315113732.GA364@elgon.mountain> (raw)

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 <dan.carpenter@oracle.com>
---
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;
 

             reply	other threads:[~2012-03-15 11:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-15 11:37 Dan Carpenter [this message]
2012-03-15 11:37 ` [patch] max17042_battery: fix a couple buffer overflows Dan Carpenter
2012-03-15 16:57 ` Dirk Brandewie
2012-03-15 16:57   ` Dirk Brandewie
2012-03-15 17:20 ` Dirk Brandewie
2012-03-15 17:20   ` Dirk Brandewie
2012-05-05  2:26   ` Anton Vorontsov
2012-05-05  2:26     ` Anton Vorontsov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120315113732.GA364@elgon.mountain \
    --to=dan.carpenter@oracle.com \
    --cc=anton.vorontsov@linaro.org \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=dirk.brandewie@gmail.com \
    --cc=grant.likely@secretlab.ca \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=myungjoo.ham@samsung.com \
    --cc=prakity@marvell.com \
    --cc=rob.herring@calxeda.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.