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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 15 Jul 2021 23:27:44 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     wuguanghao <wuguanghao3@...wei.com>
Cc:     linux-ext4@...r.kernel.org, artem.blagodarenko@...il.com,
        liuzhiqiang26@...wei.com, linfeilong@...wei.com
Subject: Re: [PATCH v2 04/12] ss_add_info_dir: fix memory leak and check
 whether

On Wed, Jun 30, 2021 at 04:27:16PM +0800, wuguanghao wrote:
> In ss_add_info_dir(), need free info->info_dirs before return,
> otherwise it will cause memory leak. At the same time, it is necessary
> to check whether dirs[n_dirs] is a null pointer, otherwise a segmentation
> fault will occur.
> 
> Signed-off-by: Wu Guanghao <wuguanghao3@...wei.com>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@...wei.com>
> Reviewed-by: Wu Bo <wubo40@...wei.com>
> ---
>  lib/ss/help.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/ss/help.c b/lib/ss/help.c
> index 5204401b..6768b9b1 100644
> --- a/lib/ss/help.c
> +++ b/lib/ss/help.c
> @@ -148,6 +148,7 @@ void ss_add_info_dir(int sci_idx, char *info_dir, int *code_ptr)
>      dirs = (char **)realloc((char *)dirs,
>  			    (unsigned)(n_dirs + 2)*sizeof(char *));
>      if (dirs == (char **)NULL) {
> +	free(info->info_dirs);
>  	info->info_dirs = (char **)NULL;
>  	*code_ptr = errno;
>  	return;

Adding the free() isn't right fix.  The real problem is that this line
should be removed:

  	info->info_dirs = (char **)NULL;

The function is trying to add a string (a directory name) to list, and
the realloc() is trying to extend the list.  If the realloc fils, we
shouldn't be zapping the original list.  We should just be returning,
and leaving the original list of directories unchanged and untouched.

    	    		      	 	     - Ted

Powered by blists - more mailing lists