On Thu, Aug 10, 2017 at 10:22:24AM +0800, Xin Long wrote:
> Commit 4440a2ab3b9f ("netfilter: synproxy: Check oom when adding synproxy
> and seqadj ct extensions") wanted to drop the packet when it fails to add
> seqadj ext due to no memory by checking if nfct_seqadj_ext_add returns
> NULL.
>
> But that nfct_seqadj_ext_add returns NULL can also happen when seqadj ext
> already exists in a nf_conn. It will cause that userspace protocol doesn't
> work when both dnat and snat are configured.
>
> Li Shuang found this issue in the case:
>
> Topo:
> ftp client router ftp server
> 10.167.131.2 <-> 10.167.131.254 10.167.141.254 <-> 10.167.141.1
>
> Rules:
> # iptables -t nat -A PREROUTING -i eth1 -p tcp -m tcp --dport 21 -j \
> DNAT --to-destination 10.167.141.1
> # iptables -t nat -A POSTROUTING -o eth2 -p tcp -m tcp --dport 21 -j \
> SNAT --to-source 10.167.141.254
>
> In router, when both dnat and snat are added, nf_nat_setup_info will be
> called twice. The packet can be dropped at the 2nd time for DNAT due to
> seqadj ext is already added at the 1st time for SNAT.
>
> This patch is to fix it by checking for seqadj ext existence before adding
> it, so that the packet will not be dropped if seqadj ext already exists.
Applied, thanks.
> Note that as Florian mentioned, as a long term, we should review ext_add()
> behaviour, it's better to return a pointer to the existing ext instead.
That seems very reasonable to me, it would be good to see if a change
like that simplifies things.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
