| Message ID | 20180225202918.538-1-ja@ssi.bg |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [net] ipvs: remove IPS_NAT_MASK check to fix passive FTP | expand |
On Sun, Feb 25, 2018 at 10:29:18PM +0200, Julian Anastasov wrote: > The IPS_NAT_MASK check in 4.12 replaced previous check for nfct_nat() > which was needed to fix a crash in 2.6.36-rc, see > commit 7bcbf81a2296 ("ipvs: avoid oops for passive FTP"). > But as IPVS does not set the IPS_SRC_NAT and IPS_DST_NAT bits, > checking for IPS_NAT_MASK prevents PASV response to be properly > mangled and blocks the transfer. Remove the check as it is not > needed after 3.12 commit 41d73ec053d2 ("netfilter: nf_conntrack: > make sequence number adjustments usuable without NAT") which > changes nfct_nat() with nfct_seqadj() and especially after 3.13 > commit b25adce16064 ("ipvs: correct usage/allocation of seqadj > ext in ipvs"). > > Thanks to Li Shuang and Florian Westphal for reporting the problem! > > Reported-by: Li Shuang <shuali@redhat.com> > Fixes: be7be6e161a2 ("netfilter: ipvs: fix incorrect conflict resolution") > Signed-off-by: Julian Anastasov <ja@ssi.bg> Acked-by: Simon Horman <horms@verge.net.au> -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 28, 2018 at 04:25:38PM +0100, Simon Horman wrote: > On Sun, Feb 25, 2018 at 10:29:18PM +0200, Julian Anastasov wrote: > > The IPS_NAT_MASK check in 4.12 replaced previous check for nfct_nat() > > which was needed to fix a crash in 2.6.36-rc, see > > commit 7bcbf81a2296 ("ipvs: avoid oops for passive FTP"). > > But as IPVS does not set the IPS_SRC_NAT and IPS_DST_NAT bits, > > checking for IPS_NAT_MASK prevents PASV response to be properly > > mangled and blocks the transfer. Remove the check as it is not > > needed after 3.12 commit 41d73ec053d2 ("netfilter: nf_conntrack: > > make sequence number adjustments usuable without NAT") which > > changes nfct_nat() with nfct_seqadj() and especially after 3.13 > > commit b25adce16064 ("ipvs: correct usage/allocation of seqadj > > ext in ipvs"). > > > > Thanks to Li Shuang and Florian Westphal for reporting the problem! > > > > Reported-by: Li Shuang <shuali@redhat.com> > > Fixes: be7be6e161a2 ("netfilter: ipvs: fix incorrect conflict resolution") > > Signed-off-by: Julian Anastasov <ja@ssi.bg> > > Acked-by: Simon Horman <horms@verge.net.au> Applied, thanks! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c index 3e17d32..58d5d05 100644 --- a/net/netfilter/ipvs/ip_vs_ftp.c +++ b/net/netfilter/ipvs/ip_vs_ftp.c @@ -260,7 +260,7 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp, buf_len = strlen(buf); ct = nf_ct_get(skb, &ctinfo); - if (ct && (ct->status & IPS_NAT_MASK)) { + if (ct) { bool mangled; /* If mangling fails this function will return 0
The IPS_NAT_MASK check in 4.12 replaced previous check for nfct_nat() which was needed to fix a crash in 2.6.36-rc, see commit 7bcbf81a2296 ("ipvs: avoid oops for passive FTP"). But as IPVS does not set the IPS_SRC_NAT and IPS_DST_NAT bits, checking for IPS_NAT_MASK prevents PASV response to be properly mangled and blocks the transfer. Remove the check as it is not needed after 3.12 commit 41d73ec053d2 ("netfilter: nf_conntrack: make sequence number adjustments usuable without NAT") which changes nfct_nat() with nfct_seqadj() and especially after 3.13 commit b25adce16064 ("ipvs: correct usage/allocation of seqadj ext in ipvs"). Thanks to Li Shuang and Florian Westphal for reporting the problem! Reported-by: Li Shuang <shuali@redhat.com> Fixes: be7be6e161a2 ("netfilter: ipvs: fix incorrect conflict resolution") Signed-off-by: Julian Anastasov <ja@ssi.bg> --- net/netfilter/ipvs/ip_vs_ftp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)