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

[Peter Maydell]

On 19 October 2018 at 04:22, Jason Wang <address@hidden> wrote:
> From: Zhang Chen <address@hidden>
>
> 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