From mboxrd@z Thu Jan  1 00:00:00 1970
From: Pablo Neira Ayuso <pablo@netfilter.org>
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 <ying.xue@windriver.com>
Return-path: <netfilter-devel-owner@vger.kernel.org>
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: netdev.vger.kernel.org

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.