linux-kernel - Re: [PATCH v2] acerhdf: fix resource reclaim in error path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <cone.1278783712.579501.3926.1000@deskpiie>
Date:	Sat, 10 Jul 2010 19:41:52 +0200
From:	Peter Feuerer <peter@...e.net>
To:	Borislav Petkov <bp@...en8.de>
Cc:	Axel Lin <axel.lin@...il.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Matthew Garrett <mjg@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Borislav Petkov <petkovbb@...il.com>,
	Alexey Dobriyan <adobriyan@...il.com>,
	platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v2] acerhdf: fix resource reclaim in error path

Borislav Petkov writes:

> From: Axel Lin <axel.lin@...il.com>
> Date: Wed, Jul 07, 2010 at 04:39:31PM +0800
> 
>> This patch fix resource reclaim in below cases:
>> 1. acerhdf_register_platform() does not properly handle
>>    platform_device_alloc() failure and platform_device_add() failure
>>    This patch adds error handing for acerhdf_register_platform().
>> 2. acerhdf_register_platform() return err with acerhdf_dev == NULL.
>>    as a result, acerhdf_unregister_platform() does not do resource
>>    reclaim in acerhdf_init() error path.
>>    This patch adds error handing for acerhdf_register_platform(),
>>    thus correct the error handing path in acerhdf_init().
>>    goto out_err instead of err_unreg if acerhdf_register_platform() fail.
>> 3. platform_device_del() should only used in error handling.
>>    Current implementation missed a platform_device_put() in acerhdf_exit.
>>    This patch fixes it by using platform_device_unregister() instead of
>>    platform_device_del() in acerhdf_unregister_platform.
>> 
>> Signed-off-by: Axel Lin <axel.lin@...il.com>
> 
> Ok, I just gave it a run and it looks good. Just a minor nitpick: when
> writing your commit messages you don't have to explain what the code
> does since we can see it in the patch itself. Rather, your commit
> message should explain why you do this.
> 
> See http://userweb.kernel.org/~akpm/stuff/tpp.txt for further info on
> how to create your patches.
> 
> Other than that:
> 
> Acked-by: Borislav Petkov <bp@...en8.de>

Acked-by: Peter Feuerer <peter@...e.net>

> 
>> ---
>>  drivers/platform/x86/acerhdf.c |   23 ++++++++++++++++-------
>>  1 files changed, 16 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
>> index 7b2384d..d9f5ebb 100644
>> --- a/drivers/platform/x86/acerhdf.c
>> +++ b/drivers/platform/x86/acerhdf.c
>> @@ -579,17 +579,26 @@ static int acerhdf_register_platform(void)
>>  		return err;
>>  
>>  	acerhdf_dev = platform_device_alloc("acerhdf", -1);
>> -	platform_device_add(acerhdf_dev);
>> +	if (!acerhdf_dev) {
>> +		err = -ENOMEM;
>> +		goto err_device_alloc;
>> +	}
>> +	err = platform_device_add(acerhdf_dev);
>> +	if (err)
>> +		goto err_device_add;
>>  
>>  	return 0;
>> +
>> +err_device_add:
>> +	platform_device_put(acerhdf_dev);
>> +err_device_alloc:
>> +	platform_driver_unregister(&acerhdf_driver);
>> +	return err;
>>  }
>>  
>>  static void acerhdf_unregister_platform(void)
>>  {
>> -	if (!acerhdf_dev)
>> -		return;
>> -
>> -	platform_device_del(acerhdf_dev);
>> +	platform_device_unregister(acerhdf_dev);
>>  	platform_driver_unregister(&acerhdf_driver);
>>  }
>>  
>> @@ -633,7 +642,7 @@ static int __init acerhdf_init(void)
>>  
>>  	err = acerhdf_register_platform();
>>  	if (err)
>> -		goto err_unreg;
>> +		goto out_err;
>>  
>>  	err = acerhdf_register_thermal();
>>  	if (err)
>> @@ -646,7 +655,7 @@ err_unreg:
>>  	acerhdf_unregister_platform();
>>  
>>  out_err:
>> -	return -ENODEV;
>> +	return err;
>>  }
>>  
>>  static void __exit acerhdf_exit(void)
>> -- 
>> 1.5.4.3

thank you,
--peter;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/