| Message ID | 1573630441-13937-1-git-send-email-lirongqing@baidu.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [v2] netfilter: only call csum_tcpudp_magic for TCP/UDP packets | expand |
Hi, On Wed, Nov 13, 2019 at 03:34:01PM +0800, Li RongQing wrote: > csum_tcpudp_magic should not be called to compute checksum > for non-TCP/UDP packets, like ICMP with wrong checksum > > Fixes: 5d1549847c76 ("netfilter: Fix remainder of pseudo-header protocol 0") > Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org> > Signed-off-by: Li RongQing <lirongqing@baidu.com> > --- > diff with v1: > rewrite the code as suggested > add fixes tag > > net/netfilter/utils.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/net/netfilter/utils.c b/net/netfilter/utils.c > index 51b454d8fa9c..0f78416566fa 100644 > --- a/net/netfilter/utils.c > +++ b/net/netfilter/utils.c > @@ -17,12 +17,21 @@ __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, > - skb->len - dataoff, protocol, > - skb->csum)) { Could you describe what you observe there to tag this patch as a Fix? Thanks.
> > - skb->len - dataoff, protocol, > > - skb->csum)) { > > Could you describe what you observe there to tag this patch as a Fix? > nothing, this fix tag can be dropped I find this unreasonable codes when I read, it should have little negative effect. thanks -Li
diff with v1: rewrite the code as suggested add fixes tag net/netfilter/utils.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/net/netfilter/utils.c b/net/netfilter/utils.c index 51b454d8fa9c..0f78416566fa 100644 --- a/net/netfilter/utils.c +++ b/net/netfilter/utils.c @@ -17,12 +17,21 @@ __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, - skb->len - dataoff, protocol, - skb->csum)) { - skb->ip_summed = CHECKSUM_UNNECESSARY; + 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; + return 0; + } + break; + default: + if (!csum_fold(skb->csum)) { + skb->ip_summed = CHECKSUM_UNNECESSARY; + return 0; + } break; } /* fall through */
csum_tcpudp_magic should not be called to compute checksum for non-TCP/UDP packets, like ICMP with wrong checksum Fixes: 5d1549847c76 ("netfilter: Fix remainder of pseudo-header protocol 0") Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Li RongQing <lirongqing@baidu.com> ---