Re: [Qemu-devel] [PULL V2 01/26] filter-rewriter: Add TCP state machine and fix memory leak in connection_track_table

[Jason Wang <jasowang@redhat.com>]

On 2018/10/29 下午7:01, Peter Maydell wrote:
> On 19 October 2018 at 04:22, Jason Wang <jasowang@redhat.com> wrote:
>> From: Zhang Chen <zhangckid@gmail.com>
>>
>> We add almost full TCP state machine in filter-rewriter, except
>> TCPS_LISTEN and some simplify in VM active close FIN states.
>> The reason for this simplify job is because guest kernel will track
>> the TCP status and wait 2MSL time too, if client resend the FIN packet,
>> guest will resend the last ACK, so we needn't wait 2MSL time in filter-rewriter.
>>
>> After a net connection is closed, we didn't clear its related resources
>> in connection_track_table, which will lead to memory leak.
>>
>> Let's track the state of net connection, if it is closed, its related
>> resources will be cleared up.
> Hi. Coverity (CID 1396477) points out that here:
>
>> +        /*
>> +         * Active close step 2.
>> +         */
>> +        if (conn->tcp_state == TCPS_FIN_WAIT_1) {
>> +            conn->tcp_state = TCPS_TIME_WAIT;
> ...this assignment to conn->tcp_state has no effect, because...
>
>> +            /*
>> +             * For simplify implementation, we needn't wait 2MSL time
>> +             * in filter rewriter. Because guest kernel will track the
>> +             * TCP status and wait 2MSL time, if client resend the FIN
>> +             * packet, guest will apply the last ACK too.
>> +             */
>> +            conn->tcp_state = TCPS_CLOSED;
> ...we immediately overwrite it with a different value.
>
>> +            g_hash_table_remove(rf->connection_track_table, key);
>> +        }
>>       }
> What was the intention of the code here?
>
> thanks
> -- PMM


Looks not.

Chen, can you please send a patch to fix this?

Thanks