2011-05-11 02:23:20

by Steven Liu

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

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 <[email protected]>
---
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

2011-05-11 02:45:50

by Steven Liu

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

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 <[email protected]>
---
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;
}

--

2011-05-11 03:03:55

by Joonyoung Shim

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

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.

2011-05-11 21:12:48

by Dmitry Torokhov

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

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