[PATCH 0/1] binder: free memory on error

[PATCH 0/1] binder: free memory on error

[Christian Brauner]

On binder_init() the devices string is duplicated and smashed into individual
device names which are passed along. However, if I'm not mistaken the original
duplicated string wasn't freed in case binder_init() failed.

Christian

Christian Brauner (1):
  binder: free memory on error

 drivers/android/binder.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.13.3

[PATCH 1/1] binder: free memory on error

[Christian Brauner]

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 drivers/android/binder.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index f7665c31feca..41a11661eed7 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -4215,7 +4215,7 @@ static int __init init_binder_device(const char *name)
 static int __init binder_init(void)
 {
 	int ret;
-	char *device_name, *device_names;
+	char *device_name, *device_names, *device_tmp;
 	struct binder_device *device;
 	struct hlist_node *tmp;
 
@@ -4263,7 +4263,8 @@ static int __init binder_init(void)
 	}
 	strcpy(device_names, binder_devices_param);
 
-	while ((device_name = strsep(&device_names, ","))) {
+	device_tmp = device_names;
+	while ((device_name = strsep(&device_tmp, ","))) {
 		ret = init_binder_device(device_name);
 		if (ret)
 			goto err_init_binder_device_failed;
@@ -4279,6 +4280,7 @@ static int __init binder_init(void)
 	}
 err_alloc_device_names_failed:
 	debugfs_remove_recursive(binder_debugfs_dir_entry_root);
+	kfree(device_names);
 
 	return ret;
 }
-- 
2.13.3

Re: [PATCH 1/1] binder: free memory on error

[Dan Carpenter]

You need a commit message.

On Mon, Aug 21, 2017 at 03:36:16PM +0200, Christian Brauner wrote:
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
>  drivers/android/binder.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index f7665c31feca..41a11661eed7 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -4215,7 +4215,7 @@ static int __init init_binder_device(const char *name)
>  static int __init binder_init(void)
>  {
>  	int ret;
> -	char *device_name, *device_names;
> +	char *device_name, *device_names, *device_tmp;
>  	struct binder_device *device;
>  	struct hlist_node *tmp;
>  
> @@ -4263,7 +4263,8 @@ static int __init binder_init(void)
>  	}
>  	strcpy(device_names, binder_devices_param);
>  
> -	while ((device_name = strsep(&device_names, ","))) {
> +	device_tmp = device_names;
> +	while ((device_name = strsep(&device_tmp, ","))) {
>  		ret = init_binder_device(device_name);
>  		if (ret)
>  			goto err_init_binder_device_failed;
> @@ -4279,6 +4280,7 @@ static int __init binder_init(void)
>  	}
>  err_alloc_device_names_failed:
>  	debugfs_remove_recursive(binder_debugfs_dir_entry_root);
> +	kfree(device_names);

Unwind in the opposite order from how things are allocated.  So this
should be added before the err_alloc_device_names_failed label:


  5293                  if (ret)
  5294                          goto err_init_binder_device_failed;
  5295          }
  5296  
  5297          return ret;
  5298  
  5299  err_init_binder_device_failed:
  5300          hlist_for_each_entry_safe(device, tmp, &binder_devices, hlist) {
  5301                  misc_deregister(&device->miscdev);
  5302                  hlist_del(&device->hlist);
  5303                  kfree(device);
  5304          }

+		kfree(device_names);

  5305  err_alloc_device_names_failed:
  5306          debugfs_remove_recursive(binder_debugfs_dir_entry_root);
  5307  
  5308          return ret;
  5309  }


regards,
dan carpenter