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@xxxxxxxxxx>
>> Signed-off-by: lixiaokeng <lixiaokeng@xxxxxxxxxx>
>> ---
>> 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
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
