On 8/13/19 10:51 AM, Pablo Neira Ayuso wrote:
> TCP rst and fin packets do not qualify to place a flow into the
> flowtable. Most likely there will be no more packets after connection
> closure. Without this patch, this flow entry expires and connection
> tracking picks up the entry in ESTABLISHED state using the fixup
> timeout, which makes this look inconsistent to the user for a connection
> that is actually already closed.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
> net/netfilter/nft_flow_offload.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
> index aa5f571d4361..6dc54c2ca856 100644
> --- a/net/netfilter/nft_flow_offload.c
> +++ b/net/netfilter/nft_flow_offload.c
> @@ -73,10 +73,10 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
> struct nft_flow_offload *priv = nft_expr_priv(expr);
> struct nf_flowtable *flowtable = &priv->flowtable->data;
> enum ip_conntrack_info ctinfo;
> + struct tcphdr *tcph = NULL;
> struct nf_flow_route route;
> struct flow_offload *flow;
> enum ip_conntrack_dir dir;
> - bool is_tcp = false;
> struct nf_conn *ct;
> int ret;
>
> @@ -89,7 +89,9 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
>
> switch (ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum) {
> case IPPROTO_TCP:
> - is_tcp = true;
> + tcph = (void *)(skb_network_header(pkt->skb) + pkt->xt.thoff);
Don't you need something like :
tcph = skb_header_pointer(pkt->skb, pkt->xt.thoff, sizeof(*tcph), buffer);
> + if (unlikely(tcph->fin || tcph->rst))
> + goto out;
> break;
> case IPPROTO_UDP:
> break;
> @@ -115,7 +117,7 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
> if (!flow)
> goto err_flow_alloc;
>
> - if (is_tcp) {
> + if (tcph) {
> ct->proto.tcp.seen[0].flags |= IP_CT_TCP_FLAG_BE_LIBERAL;
> ct->proto.tcp.seen[1].flags |= IP_CT_TCP_FLAG_BE_LIBERAL;
> }
>
