linux-ext4 - Re: [PATCH v2 04/12] ss_add_info_dir: fix memory leak and check whether

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] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 17 Jul 2021 11:10:01 +0800
From:   Zhiqiang Liu <liuzhiqiang26@...wei.com>
To:     "Theodore Y. Ts'o" <tytso@....edu>,
        wuguanghao <wuguanghao3@...wei.com>
CC:     <linux-ext4@...r.kernel.org>, <artem.blagodarenko@...il.com>,
        <linfeilong@...wei.com>
Subject: Re: [PATCH v2 04/12] ss_add_info_dir: fix memory leak and check
 whether


On 2021/7/16 11:27, Theodore Y. Ts'o wrote:
> 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

Thanks for your patient reply.

We will correct that in v3 patch.

>
> .