[dm-devel] [PATCH V4 11/14] mpathpersist: check whether malloc paramp->trnptid_list fails in handle_args func

[lixiaokeng]

On 2020/9/11 2:48, Martin Wilck wrote:
> On Thu, 2020-09-10 at 18:52 +0800, lixiaokeng wrote:
>> In handle_args func, we donot check whether malloc paramp and
>> each paramp->trnptid_list[j] fails before using them, it may
>> cause access NULL pointer.
>>
>> Here, we add alloc_prout_param_descriptor to allocate and init
>> paramp, and we add free_prout_param_descriptor to free paramp
>> and each paramp->trnptid_list[j].
>>
>> We change num_transport to num_transportids to combine them.
>>
>> Signed-off-by: Zhiqiang Liu <liuzhiqiang26 at huawei.com>
>> Signed-off-by: lixiaokeng <lixiaokeng at huawei.com>
>> ---
>>  mpathpersist/main.c | 65 ++++++++++++++++++++++++++++++++++---------
>> --
>>  1 file changed, 50 insertions(+), 15 deletions(-)
>>
>> diff --git a/mpathpersist/main.c b/mpathpersist/main.c
>> index 28bfe410..da67c15c 100644
>> --- a/mpathpersist/main.c
>> +++ b/mpathpersist/main.c
>> @@ -153,6 +153,38 @@ static int do_batch_file(const char *batch_fn)
>>  	return ret;
>>  }
>>
>> +static struct prout_param_descriptor *
>> +alloc_prout_param_descriptor(int num_transportid)
>> +{
>> +	struct prout_param_descriptor *paramp;
>> +
>> +	if (num_transportid < 0 || num_transportid > MPATH_MX_TIDS)
>> +		return NULL;
>> +
>> +	paramp= malloc(sizeof(struct prout_param_descriptor) +
>> +				(sizeof(struct transportid *) *
>> num_transportid));
>> +
>> +	if (!paramp)
>> +		return NULL;
>> +
>> +	paramp->num_transportid = num_transportid;
>> +	memset(paramp, 0, sizeof(struct prout_param_descriptor) +
>> +			(sizeof(struct transportid *) *
>> num_transportid));
>> +	return paramp;
>> +}
>> +
>> +static void free_prout_param_descriptor(struct
>> prout_param_descriptor *paramp)
>> +{
>> +	int i;
>> +	if (!paramp)
>> +		return;
>> +
>> +	for (i = 0; i < paramp->num_transportid; i++)
>> +		free(paramp->trnptid_list[i]);
> 
> This causes a compilation error. Didn't you compile-test?
> 
> main.c: In function ‘free_prout_param_descriptor’:
> main.c:182:16: error: comparison of integer expressions of different
> signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Werror=si]
>   182 |  for (i = 0; i < paramp->num_transportid; i++)
>       |                ^
> cc1: all warnings being treated as errors
> 
> Regards,
> Martin
> 
Hi Martin,
  When I compile it, I add a patch thats changes int to unsigned int.
But I don't think it is an error. It is  just a warning and becomes an
error with [-Werror]. Anyway, I will change int to unsigned it and send
it again. Thanks.

Regards
Lixiaokeng