Re: [PATCH] migration/rdma: Fix cm_event used before being initialized

[address@hidden]

On 17/05/2021 18.00, Dr. David Alan Gilbert wrote:
> * lizhijian@fujitsu.com (lizhijian@fujitsu.com) wrote:
>>
>> On 14/05/2021 01.15, Dr. David Alan Gilbert wrote:
>>> * Li Zhijian (lizhijian@cn.fujitsu.com) wrote:
>>>> A segmentation fault was triggered when i try to abort a postcopy + rdma
>>>> migration.
>>>>
>>>> since rdma_ack_cm_event releases a uninitialized cm_event in thise case.
>>>>
>>>> like below:
>>>> 2496     ret = rdma_get_cm_event(rdma->channel, &cm_event);
>>>> 2497     if (ret) {
>>>> 2498         perror("rdma_get_cm_event after rdma_connect");
>>>> 2499         ERROR(errp, "connecting to destination!");
>>>> 2500         rdma_ack_cm_event(cm_event); <<<< cause segmentation fault
>>>> 2501         goto err_rdma_source_connect;
>>>> 2502     }
>>>>
>>>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>>> OK, that's an easy fix then; but I wonder if we should perhaps remove
>>> that rdma_ack_cm_event, if it's the get_cm_event that's failed?
>> I also wondered, i checked the man page get_cm_event(3) which has not 
>> documented
>>
>> and checked some rdma examples, some of them try to ack it[1],  but some 
>> not[2].
> I think they're actually consistent:
You are right.
I also checked rdma_get_cm_even() code, indeed, event will be changed only if 
rdma_get_cm_even() returns 0.
So i agree to remove rdma_ack_cm_event(event) in error path. i will update the 
patch soon.

Thanks
Zhijian