| Message ID | BAY403-EAS3244BB5B8769FABE83B01EF954E0@phx.gbl |
|---|---|
| State | Not Applicable |
| Delegated to: | Pablo Neira |
| Headers | show |
On Tue, Sep 29, 2015 at 04:49:32PM +0800, Feng Gao wrote: > When TCP endpoint supports the windows scale option, the data size could > be more than 65536 easily. And there are some network interface features > which could aggregate multiple packets. So we need to check the datalen > before copy data into the FTP buffer. I don't think you can go over the maximum IPv4/IPv6 packet length with aggregation. -- 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 Sunday 2015-10-04 22:16, Pablo Neira Ayuso wrote: >On Tue, Sep 29, 2015 at 04:49:32PM +0800, Feng Gao wrote: >> When TCP endpoint supports the windows scale option, the data size could >> be more than 65536 easily. And there are some network interface features >> which could aggregate multiple packets. So we need to check the datalen >> before copy data into the FTP buffer. > >I don't think you can go over the maximum IPv4/IPv6 packet length with >aggregation. But there are jumbo frames known to IPv6, at least in principle. Dunno if GRO/TSO/.. do that, though. -- 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 Sun, Oct 04, 2015 at 11:38:59PM +0200, Jan Engelhardt wrote: > On Sunday 2015-10-04 22:16, Pablo Neira Ayuso wrote: > > >On Tue, Sep 29, 2015 at 04:49:32PM +0800, Feng Gao wrote: > >> When TCP endpoint supports the windows scale option, the data size could > >> be more than 65536 easily. And there are some network interface features > >> which could aggregate multiple packets. So we need to check the datalen > >> before copy data into the FTP buffer. > > > >I don't think you can go over the maximum IPv4/IPv6 packet length with > >aggregation. > > But there are jumbo frames known to IPv6, at least in principle. Dunno > if GRO/TSO/.. do that, though. I didn't find any code to aggregate IPv6 jumbograms packets at quick glance. Actually, this needs changes in the existing transport protocols to work: https://tools.ietf.org/html/rfc2675 -- 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
Yes. The features of network interface I pointed are GRO/TSO/LSO and so on. Then the kernel may aggregate one big packet. BTW, I am sorry that I don't response the email in time, because I left internet for days. -----Original Message----- From: netfilter-devel-owner@vger.kernel.org [mailto:netfilter-devel-owner@vger.kernel.org] On Behalf Of Jan Engelhardt Sent: 2015年10月5日 5:39 To: Pablo Neira Ayuso <pablo@netfilter.org> Cc: Feng Gao <gfree.wind@outlook.com>; netfilter-devel@vger.kernel.org Subject: Re: [PATCH 1/1] netfilter: ftp: Check data size before copy them into FTP buffer On Sunday 2015-10-04 22:16, Pablo Neira Ayuso wrote: >On Tue, Sep 29, 2015 at 04:49:32PM +0800, Feng Gao wrote: >> When TCP endpoint supports the windows scale option, the data size >> could be more than 65536 easily. And there are some network interface >> features which could aggregate multiple packets. So we need to check >> the datalen before copy data into the FTP buffer. > >I don't think you can go over the maximum IPv4/IPv6 packet length with >aggregation. But there are jumbo frames known to IPv6, at least in principle. Dunno if GRO/TSO/.. do that, though. -- 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 -- 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/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c index b666959..79ae8a9 100644 --- a/net/netfilter/nf_conntrack_ftp.c +++ b/net/netfilter/nf_conntrack_ftp.c @@ -35,6 +35,7 @@ MODULE_ALIAS_NFCT_HELPER("ftp"); /* This is slow, but it's simple. --RR */ static char *ftp_buffer; +#define NF_FTP_BUF_SIZE (65536) static DEFINE_SPINLOCK(nf_ftp_lock); @@ -422,6 +423,11 @@ static int help(struct sk_buff *skb, return NF_ACCEPT; } datalen = skb->len - dataoff; + if (unlikely(datalen > NF_FTP_BUF_SIZE)) { + pr_warn("ftp: Data len(%u) is more than ftp buffer(%u)\n", + datalen, NF_FTP_BUF_SIZE); + return NF_ACCEPT; + } spin_lock_bh(&nf_ftp_lock); fb_ptr = skb_header_pointer(skb, dataoff, datalen, ftp_buffer); @@ -600,7 +606,7 @@ static int __init nf_conntrack_ftp_init(void) { int i, j = -1, ret = 0; - ftp_buffer = kmalloc(65536, GFP_KERNEL); + ftp_buffer = kmalloc(NF_FTP_BUF_SIZE, GFP_KERNEL); if (!ftp_buffer) return -ENOMEM;
When TCP endpoint supports the windows scale option, the data size could be more than 65536 easily. And there are some network interface features which could aggregate multiple packets. So we need to check the datalen before copy data into the FTP buffer. Signed-off-by: Feng Gao <fgao@ikuai8.com> --- net/netfilter/nf_conntrack_ftp.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)