[PATCH] touchscreen: check kzalloc memory for data first

[PATCH] touchscreen: check kzalloc memory for data first

[Steven Liu]

Hi Joonyoung Shim ,

            Can this patch be fixed in?


if kzalloc memroy for data was faild, it will multi free data
memory space. so check the data memory first, is it kzalloc faild
for data, it should goto end and do nothing.

Signed-off-by: LiuQi <lingjiujianke@gmail.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
b/drivers/input/touchscreen/atmel_mxt_ts.c
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1039,8 +1039,15 @@ static int __devinit mxt_probe(struct i2c_client *client,
 		return -EINVAL;

 	data = kzalloc(sizeof(struct mxt_data), GFP_KERNEL);
+
+	if(!data){
+		dev_err(&client->dev, "Failed to allocate memory\n");
+		error = -ENOMEM;
+		goto err_alloc_data_mem_faild;
+	}
+
 	input_dev = input_allocate_device();
-	if (!data || !input_dev) {
+	if (!input_dev) {
 		dev_err(&client->dev, "Failed to allocate memory\n");
 		error = -ENOMEM;
 		goto err_free_mem;
@@ -1107,9 +1114,10 @@ err_free_irq:
 	free_irq(client->irq, data);
 err_free_object:
 	kfree(data->object_table);
-err_free_mem:
 	input_free_device(input_dev);
+err_free_mem:
 	kfree(data);
+err_alloc_data_mem_faild:
 	return error;
 }







Best Regards

Steven Liu

Re: [PATCH] touchscreen: check kzalloc memory for data first

[Steven Liu]

fixed the code style


new patch is



if kzalloc memroy for data was faild, it will multi free data
memory space. so check the data memory first, is it kzalloc faild
for data, it should goto end and do nothing.

Signed-off-by: LiuQi <liuqi@thunderst.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
b/drivers/input/touchscreen/atmel_mxt_ts.c
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1039,8 +1039,14 @@ static int __devinit mxt_probe(struct i2c_client *client,
 		return -EINVAL;

 	data = kzalloc(sizeof(struct mxt_data), GFP_KERNEL);
+	if (!data) {
+		dev_err(&client->dev, "Failed to allocate memory\n");
+		error = -ENOMEM;
+		goto err_alloc_data_mem_faild;
+	}
+
 	input_dev = input_allocate_device();
-	if (!data || !input_dev) {
+	if (!input_dev) {
 		dev_err(&client->dev, "Failed to allocate memory\n");
 		error = -ENOMEM;
 		goto err_free_mem;
@@ -1107,9 +1113,10 @@ err_free_irq:
 	free_irq(client->irq, data);
 err_free_object:
 	kfree(data->object_table);
-err_free_mem:
 	input_free_device(input_dev);
+err_free_mem:
 	kfree(data);
+err_alloc_data_mem_faild:
 	return error;
 }

--

Re: [PATCH] touchscreen: check kzalloc memory for data first

[Joonyoung Shim]

Hi, Steven.

On 2011-05-11 오전 11:23, Steven Liu wrote:
> Hi Joonyoung Shim ,
>
>              Can this patch be fixed in?
>
>
> if kzalloc memroy for data was faild, it will multi free data
> memory space. so check the data memory first, is it kzalloc faild
> for data, it should goto end and do nothing.
>

Hmm, I don't think this patch needs. If you see the codes, there is no
multi free data memory, just free data and input_dev. This both free
are no problem because when the pointer is NULL, no operation is
perfomed in the free function.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] touchscreen: check kzalloc memory for data first

[Dmitry Torokhov]

Hi Steven,

On Wed, May 11, 2011 at 10:23:16AM +0800, Steven Liu wrote:
> Hi Joonyoung Shim ,
> 
>             Can this patch be fixed in?
> 
> 
> if kzalloc memroy for data was faild, it will multi free data
> memory space. so check the data memory first, is it kzalloc faild
> for data, it should goto end and do nothing.

There is no double-free as both kfree() and input_free_device() know to
ignore NULL pointers and we ensure that both pointers to be initialized
(either point to allocated memory or be NULL). We do not expect either
of the memory allocations to fail so instead of handling failures
separately they are handled both at once.

Thanks.

-- 
Dmitry