Re: [ndctl PATCH 2/2] namespace: fix potentail fd leak problem in do_xaction_namespace()

[Zhiqiang Liu <liuzhiqiang26@huawei.com>]

On 2021/7/1 1:47, Alison Schofield wrote:
> On Tue, Jun 15, 2021 at 08:39:20PM +0800, Zhiqiang Liu wrote:
>> In do_xaction_namespace(), ri_ctx.f_out should be closed after
>> being opened.
>>
> Hi Zhiqiang,
>
> The commit message and commit log need to be swapped.
>
> Something like:
>
> Commit message says what the patch does:
> [ndctl PATCH 2/2] namespace: Close fd before return in do_xaction_namespace()
>
> Commit log says why it needs to be done:
> This prevents a potential file descriptor leak in do_xaction_namespace()
>
> And, same as in Patch 1 - mention it was found by Coverity.
>
> Alison

Thanks for your advice.

I will do that as your suggestion in v2 patch.



>
>> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
>> ---
>>  ndctl/namespace.c | 17 +++++++++--------
>>  1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
>> index 21089d7..55364ac 100644
>> --- a/ndctl/namespace.c
>> +++ b/ndctl/namespace.c
>> @@ -2141,7 +2141,7 @@ static int do_xaction_namespace(const char *namespace,
>>  				util_display_json_array(ri_ctx.f_out, ri_ctx.jblocks, 0);
>>  			if (rc >= 0)
>>  				(*processed)++;
>> -			return rc;
>> +			goto out;
>>  		}
>>  	}
>>
>> @@ -2152,11 +2152,11 @@ static int do_xaction_namespace(const char *namespace,
>>  		rc = file_write_infoblock(param.outfile);
>>  		if (rc >= 0)
>>  			(*processed)++;
>> -		return rc;
>> +		goto out;
>>  	}
>>
>>  	if (!namespace && action != ACTION_CREATE)
>> -		return rc;
>> +		goto out;
>>
>>  	if (verbose)
>>  		ndctl_set_log_priority(ctx, LOG_DEBUG);
>> @@ -2212,7 +2212,7 @@ static int do_xaction_namespace(const char *namespace,
>>  						saved_rc = rc;
>>  						continue;
>>  				}
>> -				return rc;
>> +				goto out;
>>  			}
>>  			ndctl_namespace_foreach_safe(region, ndns, _n) {
>>  				ndns_name = ndctl_namespace_get_devname(ndns);
>> @@ -2259,7 +2259,7 @@ static int do_xaction_namespace(const char *namespace,
>>  					rc = namespace_reconfig(region, ndns);
>>  					if (rc == 0)
>>  						*processed = 1;
>> -					return rc;
>> +					goto out;
>>  				case ACTION_READ_INFOBLOCK:
>>  					rc = namespace_rw_infoblock(ndns, &ri_ctx, READ);
>>  					if (rc == 0)
>> @@ -2281,9 +2281,6 @@ static int do_xaction_namespace(const char *namespace,
>>  	if (ri_ctx.jblocks)
>>  		util_display_json_array(ri_ctx.f_out, ri_ctx.jblocks, 0);
>>
>> -	if (ri_ctx.f_out && ri_ctx.f_out != stdout)
>> -		fclose(ri_ctx.f_out);
>> -
>>  	if (action == ACTION_CREATE && rc == -EAGAIN) {
>>  		/*
>>  		 * Namespace creation searched through all candidate
>> @@ -2301,6 +2298,10 @@ static int do_xaction_namespace(const char *namespace,
>>  	if (saved_rc)
>>  		rc = saved_rc;
>>
>> +out:
>> +	if (ri_ctx.f_out && ri_ctx.f_out != stdout)
>> +		fclose(ri_ctx.f_out);
>> +
>>  	return rc;
>>  }
>>
>> -- 
>> 2.23.0
>>
>>
>>
>>
>> .
>>
>>
>>
> .