On Thu, 7 Apr 2022, Florian Westphal wrote:
> Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxx> wrote:
> > I'd merge the two conditions so that it'd cover both original condition
> > branches:
> >
> > diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
> > index 8ec55cd72572..87375ce2f995 100644
> > --- a/net/netfilter/nf_conntrack_proto_tcp.c
> > +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> > @@ -556,33 +556,26 @@ static bool tcp_in_window(struct nf_conn *ct,
> > }
> >
> > }
> > - } else if (((state->state == TCP_CONNTRACK_SYN_SENT
> > - && dir == IP_CT_DIR_ORIGINAL)
> > - || (state->state == TCP_CONNTRACK_SYN_RECV
> > - && dir == IP_CT_DIR_REPLY))
> > - && after(end, sender->td_end)) {
> > + } else if (tcph->syn &&
> > + ((after(end, sender->td_end) &&
> > + (state->state == TCP_CONNTRACK_SYN_SENT ||
> > + state->state == TCP_CONNTRACK_SYN_RECV)) ||
> > + (dir == IP_CT_DIR_REPLY &&
> > + state->state == TCP_CONNTRACK_SYN_SENT))) {
>
> Thats what I did as well, I merged the two branches but I made the
> 2nd clause stricter to also consider the after() test; it would no
> longer re-init for syn-acks when sequence did not advance.
That's perfectly fine.
But what about simultaneous syn? The TCP state is zeroed in the REPLY
direction, so the after() test can easily be false and the state wouldn't
be picked up. Therefore I extended the condition.
Best regards,
Jozsef
-
E-mail : kadlec@xxxxxxxxxxxxxxxxx, kadlecsik.jozsef@xxxxxxxxx
PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics
H-1525 Budapest 114, POB. 49, Hungary
