From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linuxfoundation.org ([140.211.169.12]:46912 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752218AbeCMJ4A (ORCPT ); Tue, 13 Mar 2018 05:56:00 -0400 Date: Tue, 13 Mar 2018 10:56:00 +0100 From: Greg Kroah-Hartman To: Ben Hutchings Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Alexey Kodanev , Marcelo Ricardo Leitner , Neil Horman , "David S. Miller" Subject: Re: [PATCH 4.4 31/36] sctp: verify size of a new chunk in _sctp_make_chunk() Message-ID: <20180313095600.GA5131@kroah.com> References: <20180310001807.213987241@linuxfoundation.org> <20180310001809.079131255@linuxfoundation.org> <1520902018.23626.90.camel@codethink.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1520902018.23626.90.camel@codethink.co.uk> Sender: stable-owner@vger.kernel.org List-ID: On Tue, Mar 13, 2018 at 12:46:58AM +0000, Ben Hutchings wrote: > On Fri, 2018-03-09 at 16:18 -0800, Greg Kroah-Hartman wrote: > > 4.4-stable review patch.��If anyone has any objections, please let me know. > > > > ------------------ > > > > From: Alexey Kodanev > > > > > > [ Upstream commit 07f2c7ab6f8d0a7e7c5764c4e6cc9c52951b9d9c ] > > > > When SCTP makes INIT or INIT_ACK packet the total chunk length > > can exceed SCTP_MAX_CHUNK_LEN which leads to kernel panic when > > transmitting these packets, e.g. the crash on sending INIT_ACK: > > > > [��597.804948] skbuff: skb_over_panic: text:00000000ffae06e4 len:120168 > > ���������������put:120156 head:000000007aa47635 data:00000000d991c2de > > ���������������tail:0x1d640 end:0xfec0 dev: > > ... > > [��597.976970] ------------[ cut here ]------------ > > [��598.033408] kernel BUG at net/core/skbuff.c:104! > > [��600.314841] Call Trace: > > [��600.345829]�� > > [��600.371639]��? sctp_packet_transmit+0x2095/0x26d0 [sctp] > > [��600.436934]��skb_put+0x16c/0x200 > > [��600.477295]��sctp_packet_transmit+0x2095/0x26d0 [sctp] > > [��600.540630]��? sctp_packet_config+0x890/0x890 [sctp] > > [��600.601781]��? __sctp_packet_append_chunk+0x3b4/0xd00 [sctp] > > [��600.671356]��? sctp_cmp_addr_exact+0x3f/0x90 [sctp] > > [��600.731482]��sctp_outq_flush+0x663/0x30d0 [sctp] > > [��600.788565]��? sctp_make_init+0xbf0/0xbf0 [sctp] > > [��600.845555]��? sctp_check_transmitted+0x18f0/0x18f0 [sctp] > > [��600.912945]��? sctp_outq_tail+0x631/0x9d0 [sctp] > > [��600.969936]��sctp_cmd_interpreter.isra.22+0x3be1/0x5cb0 [sctp] > > [��601.041593]��? sctp_sf_do_5_1B_init+0x85f/0xc30 [sctp] > > [��601.104837]��? sctp_generate_t1_cookie_event+0x20/0x20 [sctp] > > [��601.175436]��? sctp_eat_data+0x1710/0x1710 [sctp] > > [��601.233575]��sctp_do_sm+0x182/0x560 [sctp] > > [��601.284328]��? sctp_has_association+0x70/0x70 [sctp] > > [��601.345586]��? sctp_rcv+0xef4/0x32f0 [sctp] > > [��601.397478]��? sctp6_rcv+0xa/0x20 [sctp] > > ... > > > > Here the chunk size for INIT_ACK packet becomes too big, mostly > > because of the state cookie (INIT packet has large size with > > many address parameters), plus additional server parameters. > > > > Later this chunk causes the panic in skb_put_data(): > > > > � skb_packet_transmit() > > ������sctp_packet_pack() > > ����������skb_put_data(nskb, chunk->skb->data, chunk->skb->len); > > > > 'nskb' (head skb) was previously allocated with packet->size > > from u16 'chunk->chunk_hdr->length'. > > > > As suggested by Marcelo we should check the chunk's length in > > _sctp_make_chunk() before trying to allocate skb for it and > > discard a chunk if its size bigger than SCTP_MAX_CHUNK_LEN. > > > > > Signed-off-by: Alexey Kodanev > > > Acked-by: Marcelo Ricardo Leitner > > > Acked-by: Neil Horman > > > Signed-off-by: David S. Miller > > > Signed-off-by: Greg Kroah-Hartman > > --- > > �net/sctp/sm_make_chunk.c |����8 ++++++-- > > �1 file changed, 6 insertions(+), 2 deletions(-) > > > > --- a/net/sctp/sm_make_chunk.c > > +++ b/net/sctp/sm_make_chunk.c > > @@ -1367,10 +1367,14 @@ static struct sctp_chunk *_sctp_make_chu > > � sctp_chunkhdr_t *chunk_hdr; > > � struct sk_buff *skb; > > � struct sock *sk; > > + int chunklen; > > + > > + chunklen = sizeof(*chunk_hdr) + paylen; > > I think this length still needs to be rounded up (with WORD_ROUND here, > instead of SCTP_PAD4 upstream). Ah, good point, how's this patch: diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index 4ca31e052dd8..509e9426a056 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -1369,7 +1369,7 @@ static struct sctp_chunk *_sctp_make_chunk(const struct sctp_association *asoc, struct sock *sk; int chunklen; - chunklen = sizeof(*chunk_hdr) + paylen; + chunklen = WORD_ROUND(sizeof(*chunk_hdr) + paylen); if (chunklen > SCTP_MAX_CHUNK_LEN) goto nodata;