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

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]

Date:   Mon, 21 Aug 2017 17:00:51 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Christian Brauner <christian.brauner@...ntu.com>
Cc:     gregkh@...uxfoundation.org, arve@...roid.com,
        riandrews@...roid.com, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] binder: free memory on error

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@...ntu.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