Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] netfilter: only call csum_tcpudp_magic for TCP/UDP packets
@ 2019-11-09  7:50 Li RongQing
  2019-11-12 21:30 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 3+ messages in thread
From: Li RongQing @ 2019-11-09  7:50 UTC (permalink / raw)
  To: netfilter-devel

csum_tcpudp_magic should not be called to compute checksum
for non-TCP/UDP packets, like ICMP with wrong checksum

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 net/netfilter/utils.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/utils.c b/net/netfilter/utils.c
index 51b454d8fa9c..72eace52874e 100644
--- a/net/netfilter/utils.c
+++ b/net/netfilter/utils.c
@@ -17,9 +17,12 @@ __sum16 nf_ip_checksum(struct sk_buff *skb, unsigned int hook,
 	case CHECKSUM_COMPLETE:
 		if (hook != NF_INET_PRE_ROUTING && hook != NF_INET_LOCAL_IN)
 			break;
-		if ((protocol != IPPROTO_TCP && protocol != IPPROTO_UDP &&
-		    !csum_fold(skb->csum)) ||
-		    !csum_tcpudp_magic(iph->saddr, iph->daddr,
+		if (protocol != IPPROTO_TCP && protocol != IPPROTO_UDP) {
+			if (!csum_fold(skb->csum)) {
+				skb->ip_summed = CHECKSUM_UNNECESSARY;
+				break;
+			}
+		} else if (!csum_tcpudp_magic(iph->saddr, iph->daddr,
 				       skb->len - dataoff, protocol,
 				       skb->csum)) {
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
-- 
2.16.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] netfilter: only call csum_tcpudp_magic for TCP/UDP packets
  2019-11-09  7:50 [PATCH] netfilter: only call csum_tcpudp_magic for TCP/UDP packets Li RongQing
@ 2019-11-12 21:30 ` Pablo Neira Ayuso
  2019-11-13  1:13   ` 答复: " Li,Rongqing
  0 siblings, 1 reply; 3+ messages in thread
From: Pablo Neira Ayuso @ 2019-11-12 21:30 UTC (permalink / raw)
  To: Li RongQing; +Cc: netfilter-devel

On Sat, Nov 09, 2019 at 03:50:17PM +0800, Li RongQing wrote:
> csum_tcpudp_magic should not be called to compute checksum
> for non-TCP/UDP packets, like ICMP with wrong checksum

This is fixing 5d1549847c76b1ffcf8e388ef4d0f229bdd1d7e8.

> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  net/netfilter/utils.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/net/netfilter/utils.c b/net/netfilter/utils.c
> index 51b454d8fa9c..72eace52874e 100644
> --- a/net/netfilter/utils.c
> +++ b/net/netfilter/utils.c
> @@ -17,9 +17,12 @@ __sum16 nf_ip_checksum(struct sk_buff *skb, unsigned int hook,
>  	case CHECKSUM_COMPLETE:
>  		if (hook != NF_INET_PRE_ROUTING && hook != NF_INET_LOCAL_IN)
>  			break;
> -		if ((protocol != IPPROTO_TCP && protocol != IPPROTO_UDP &&
> -		    !csum_fold(skb->csum)) ||
> -		    !csum_tcpudp_magic(iph->saddr, iph->daddr,
> +		if (protocol != IPPROTO_TCP && protocol != IPPROTO_UDP) {
> +			if (!csum_fold(skb->csum)) {
> +				skb->ip_summed = CHECKSUM_UNNECESSARY;
> +				break;
> +			}
> +		} else if (!csum_tcpudp_magic(iph->saddr, iph->daddr,
>  				       skb->len - dataoff, protocol,
>  				       skb->csum)) {

Probably disentangle this code with the following snippet?

                switch (protocol) {
                case IPPROTO_TCP:
                case IPPROTO_UDP:
                        if (!csum_tcpudp_magic(iph->saddr, iph->daddr,
                                               skb->len - dataoff, protocol,
                                               skb->csum))
                                 skb->ip_summed = CHECKSUM_UNNECESSARY;
                        break;
                default:
                        if (!csum_fold(skb->csum))
                                skb->ip_summed = CHECKSUM_UNNECESSARY;
                        break;
                }

> +                     if (!csum_fold(skb->csum)) {                             
> +                             skb->ip_summed = CHECKSUM_UNNECESSARY;           
> +                             break;                                           
> +                     }                                                        
> +             } else if (!csum_tcpudp_magic(iph->saddr, iph->daddr,            
>                                      skb->len - dataoff, protocol,             
>                                      skb->csum)) {
>  			skb->ip_summed = CHECKSUM_UNNECESSARY;
> -- 
> 2.16.2
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* 答复: [PATCH] netfilter: only call csum_tcpudp_magic for TCP/UDP packets
  2019-11-12 21:30 ` Pablo Neira Ayuso
@ 2019-11-13  1:13   ` " Li,Rongqing
  0 siblings, 0 replies; 3+ messages in thread
From: Li,Rongqing @ 2019-11-13  1:13 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel



> -----邮件原件-----
> 发件人: netfilter-devel-owner@vger.kernel.org
> [mailto:netfilter-devel-owner@vger.kernel.org] 代表 Pablo Neira Ayuso
> 发送时间: 2019年11月13日 5:30
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: netfilter-devel@vger.kernel.org
> 主题: Re: [PATCH] netfilter: only call csum_tcpudp_magic for TCP/UDP packets
> 
> On Sat, Nov 09, 2019 at 03:50:17PM +0800, Li RongQing wrote:
> > csum_tcpudp_magic should not be called to compute checksum for
> > non-TCP/UDP packets, like ICMP with wrong checksum
> 
> This is fixing 5d1549847c76b1ffcf8e388ef4d0f229bdd1d7e8.
> 
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >  net/netfilter/utils.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/netfilter/utils.c b/net/netfilter/utils.c index
> > 51b454d8fa9c..72eace52874e 100644
> > --- a/net/netfilter/utils.c
> > +++ b/net/netfilter/utils.c
> > @@ -17,9 +17,12 @@ __sum16 nf_ip_checksum(struct sk_buff *skb,
> unsigned int hook,
> >  	case CHECKSUM_COMPLETE:
> >  		if (hook != NF_INET_PRE_ROUTING && hook != NF_INET_LOCAL_IN)
> >  			break;
> > -		if ((protocol != IPPROTO_TCP && protocol != IPPROTO_UDP &&
> > -		    !csum_fold(skb->csum)) ||
> > -		    !csum_tcpudp_magic(iph->saddr, iph->daddr,
> > +		if (protocol != IPPROTO_TCP && protocol != IPPROTO_UDP) {
> > +			if (!csum_fold(skb->csum)) {
> > +				skb->ip_summed = CHECKSUM_UNNECESSARY;
> > +				break;
> > +			}
> > +		} else if (!csum_tcpudp_magic(iph->saddr, iph->daddr,
> >  				       skb->len - dataoff, protocol,
> >  				       skb->csum)) {
> 
> Probably disentangle this code with the following snippet?
> 
>                 switch (protocol) {
>                 case IPPROTO_TCP:
>                 case IPPROTO_UDP:
>                         if (!csum_tcpudp_magic(iph->saddr, iph->daddr,
>                                                skb->len - dataoff,
> protocol,
>                                                skb->csum))
>                                  skb->ip_summed =
> CHECKSUM_UNNECESSARY;
>                         break;
>                 default:
>                         if (!csum_fold(skb->csum))
>                                 skb->ip_summed =
> CHECKSUM_UNNECESSARY;
>                         break;
>                 }
> 
OK ,I will send V2, thanks

-RongQing

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-09  7:50 [PATCH] netfilter: only call csum_tcpudp_magic for TCP/UDP packets Li RongQing
2019-11-12 21:30 ` Pablo Neira Ayuso
2019-11-13  1:13   ` 答复: " Li,Rongqing

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org
	public-inbox-index netfilter-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git