From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pablo Neira Ayuso Subject: Re: [net] netfilter: nat: sctp: fix ICMP packet to be dropped accidently Date: Wed, 8 Mar 2017 18:29:43 +0100 Message-ID: <20170308172943.GA8577@salvia> References: <1488621602-4152-1-git-send-email-ying.xue@windriver.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netfilter-devel@vger.kernel.org, davem@davemloft.net, netdev@vger.kernel.org To: Ying Xue Return-path: Received: from mail.us.es ([193.147.175.20]:35474 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753254AbdCHRji (ORCPT ); Wed, 8 Mar 2017 12:39:38 -0500 Received: from antivirus1-rhel7.int (unknown [192.168.2.11]) by mail.us.es (Postfix) with ESMTP id 26ED31B2B7F for ; Wed, 8 Mar 2017 18:29:51 +0100 (CET) Received: from antivirus1-rhel7.int (localhost [127.0.0.1]) by antivirus1-rhel7.int (Postfix) with ESMTP id 0C739DA808 for ; Wed, 8 Mar 2017 18:29:51 +0100 (CET) Received: from antivirus1-rhel7.int (localhost [127.0.0.1]) by antivirus1-rhel7.int (Postfix) with ESMTP id CE785DA80B for ; Wed, 8 Mar 2017 18:29:48 +0100 (CET) Content-Disposition: inline In-Reply-To: <1488621602-4152-1-git-send-email-ying.xue@windriver.com> Sender: netfilter-devel-owner@vger.kernel.org List-ID: On Sat, Mar 04, 2017 at 06:00:02PM +0800, Ying Xue wrote: > Regarding RFC 792, the first 64 bits of the original SCTP datagram's > data could be contained in ICMP packet, such as: > > 0 1 2 3 > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Type | Code | Checksum | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | unused | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Internet Header + 64 bits of Original Data Datagram | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > However, according to RFC 4960, SCTP datagram header is as below: > > 0 1 2 3 > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Source Port Number | Destination Port Number | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Verification Tag | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Checksum | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > It means only the first three fields of SCTP header can be carried in > ICMP packet except for Checksum field. > > At present in sctp_manip_pkt(), no matter whether the packet is ICMP or > not, it always calculates SCTP packet checksum. However, not only the > calculation of checksum is unnecessary for ICMP, but also it causes > another fatal issue that ICMP packet is dropped. The header size of > SCTP is used to identify whether the writeable length of skb is bigger > than skb->len through skb_make_writable() in sctp_manip_pkt(). But > when it deals with ICMP packet, skb_make_writable() directly returns > false as the writeable length of skb is bigger than skb->len. > Subsequently ICMP is dropped. > > Now we correct this misbahavior. When sctp_manip_pkt() handles ICMP > packet, 8 bytes rather than the whole SCTP header size is used to check > if writeable length of skb is overflowed. Meanwhile, as it's meaningless > to calculate checksum when packet is ICMP, the computation of checksum > is ignored as well. Applied, thanks.