All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] sctp: verify size of a new chunk in _sctp_make_chunk()
@ 2018-02-09 14:35 ` Alexey Kodanev
  0 siblings, 0 replies; 8+ messages in thread
From: Alexey Kodanev @ 2018-02-09 14:35 UTC (permalink / raw)
  To: netdev
  Cc: Marcelo Ricardo Leitner, Neil Horman, Vlad Yasevich,
	David Miller, linux-sctp, Alexey Kodanev

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:<NULL>
...
[  597.976970] ------------[ cut here ]------------
[  598.033408] kernel BUG at net/core/skbuff.c:104!
[  600.314841] Call Trace:
[  600.345829]  <IRQ>
[  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 <alexey.kodanev@oracle.com>
---

v2: account for padding before checking chunklen

 net/sctp/sm_make_chunk.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 793b05e..d01475f 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1380,9 +1380,14 @@ static struct sctp_chunk *_sctp_make_chunk(const struct sctp_association *asoc,
 	struct sctp_chunk *retval;
 	struct sk_buff *skb;
 	struct sock *sk;
+	int chunklen;
+
+	chunklen = SCTP_PAD4(sizeof(*chunk_hdr) + paylen);
+	if (chunklen > SCTP_MAX_CHUNK_LEN)
+		goto nodata;
 
 	/* No need to allocate LL here, as this is only a chunk. */
-	skb = alloc_skb(SCTP_PAD4(sizeof(*chunk_hdr) + paylen), gfp);
+	skb = alloc_skb(chunklen, gfp);
 	if (!skb)
 		goto nodata;
 
-- 
1.8.3.1

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

* [PATCH net v2] sctp: verify size of a new chunk in _sctp_make_chunk()
@ 2018-02-09 14:35 ` Alexey Kodanev
  0 siblings, 0 replies; 8+ messages in thread
From: Alexey Kodanev @ 2018-02-09 14:35 UTC (permalink / raw)
  To: netdev
  Cc: Marcelo Ricardo Leitner, Neil Horman, Vlad Yasevich,
	David Miller, linux-sctp, Alexey Kodanev

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:<NULL>
...
[  597.976970] ------------[ cut here ]------------
[  598.033408] kernel BUG at net/core/skbuff.c:104!
[  600.314841] Call Trace:
[  600.345829]  <IRQ>
[  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 <alexey.kodanev@oracle.com>
---

v2: account for padding before checking chunklen

 net/sctp/sm_make_chunk.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 793b05e..d01475f 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1380,9 +1380,14 @@ static struct sctp_chunk *_sctp_make_chunk(const struct sctp_association *asoc,
 	struct sctp_chunk *retval;
 	struct sk_buff *skb;
 	struct sock *sk;
+	int chunklen;
+
+	chunklen = SCTP_PAD4(sizeof(*chunk_hdr) + paylen);
+	if (chunklen > SCTP_MAX_CHUNK_LEN)
+		goto nodata;
 
 	/* No need to allocate LL here, as this is only a chunk. */
-	skb = alloc_skb(SCTP_PAD4(sizeof(*chunk_hdr) + paylen), gfp);
+	skb = alloc_skb(chunklen, gfp);
 	if (!skb)
 		goto nodata;
 
-- 
1.8.3.1


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

* Re: [PATCH net v2] sctp: verify size of a new chunk in _sctp_make_chunk()
  2018-02-09 14:35 ` Alexey Kodanev
@ 2018-02-09 14:40   ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 8+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-02-09 14:40 UTC (permalink / raw)
  To: Alexey Kodanev
  Cc: netdev, Neil Horman, Vlad Yasevich, David Miller, linux-sctp

On Fri, Feb 09, 2018 at 05:35:23PM +0300, Alexey Kodanev wrote:
> 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:<NULL>
> ...
> [  597.976970] ------------[ cut here ]------------
> [  598.033408] kernel BUG at net/core/skbuff.c:104!
> [  600.314841] Call Trace:
> [  600.345829]  <IRQ>
> [  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 <alexey.kodanev@oracle.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leinter@gmail.com>

Thanks Alexey.

> ---
> 
> v2: account for padding before checking chunklen
> 
>  net/sctp/sm_make_chunk.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 793b05e..d01475f 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -1380,9 +1380,14 @@ static struct sctp_chunk *_sctp_make_chunk(const struct sctp_association *asoc,
>  	struct sctp_chunk *retval;
>  	struct sk_buff *skb;
>  	struct sock *sk;
> +	int chunklen;
> +
> +	chunklen = SCTP_PAD4(sizeof(*chunk_hdr) + paylen);
> +	if (chunklen > SCTP_MAX_CHUNK_LEN)
> +		goto nodata;
>  
>  	/* No need to allocate LL here, as this is only a chunk. */
> -	skb = alloc_skb(SCTP_PAD4(sizeof(*chunk_hdr) + paylen), gfp);
> +	skb = alloc_skb(chunklen, gfp);
>  	if (!skb)
>  		goto nodata;
>  
> -- 
> 1.8.3.1
> 

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

* Re: [PATCH net v2] sctp: verify size of a new chunk in _sctp_make_chunk()
@ 2018-02-09 14:40   ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 8+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-02-09 14:40 UTC (permalink / raw)
  To: Alexey Kodanev
  Cc: netdev, Neil Horman, Vlad Yasevich, David Miller, linux-sctp

On Fri, Feb 09, 2018 at 05:35:23PM +0300, Alexey Kodanev wrote:
> 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:<NULL>
> ...
> [  597.976970] ------------[ cut here ]------------
> [  598.033408] kernel BUG at net/core/skbuff.c:104!
> [  600.314841] Call Trace:
> [  600.345829]  <IRQ>
> [  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 <alexey.kodanev@oracle.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leinter@gmail.com>

Thanks Alexey.

> ---
> 
> v2: account for padding before checking chunklen
> 
>  net/sctp/sm_make_chunk.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 793b05e..d01475f 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -1380,9 +1380,14 @@ static struct sctp_chunk *_sctp_make_chunk(const struct sctp_association *asoc,
>  	struct sctp_chunk *retval;
>  	struct sk_buff *skb;
>  	struct sock *sk;
> +	int chunklen;
> +
> +	chunklen = SCTP_PAD4(sizeof(*chunk_hdr) + paylen);
> +	if (chunklen > SCTP_MAX_CHUNK_LEN)
> +		goto nodata;
>  
>  	/* No need to allocate LL here, as this is only a chunk. */
> -	skb = alloc_skb(SCTP_PAD4(sizeof(*chunk_hdr) + paylen), gfp);
> +	skb = alloc_skb(chunklen, gfp);
>  	if (!skb)
>  		goto nodata;
>  
> -- 
> 1.8.3.1
> 

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

* Re: [PATCH net v2] sctp: verify size of a new chunk in _sctp_make_chunk()
  2018-02-09 14:35 ` Alexey Kodanev
@ 2018-02-09 15:02   ` Neil Horman
  -1 siblings, 0 replies; 8+ messages in thread
From: Neil Horman @ 2018-02-09 15:02 UTC (permalink / raw)
  To: Alexey Kodanev
  Cc: netdev, Marcelo Ricardo Leitner, Vlad Yasevich, David Miller, linux-sctp

On Fri, Feb 09, 2018 at 05:35:23PM +0300, Alexey Kodanev wrote:
> 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:<NULL>
> ...
> [  597.976970] ------------[ cut here ]------------
> [  598.033408] kernel BUG at net/core/skbuff.c:104!
> [  600.314841] Call Trace:
> [  600.345829]  <IRQ>
> [  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 <alexey.kodanev@oracle.com>
> ---
> 
> v2: account for padding before checking chunklen
> 
>  net/sctp/sm_make_chunk.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 793b05e..d01475f 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -1380,9 +1380,14 @@ static struct sctp_chunk *_sctp_make_chunk(const struct sctp_association *asoc,
>  	struct sctp_chunk *retval;
>  	struct sk_buff *skb;
>  	struct sock *sk;
> +	int chunklen;
> +
> +	chunklen = SCTP_PAD4(sizeof(*chunk_hdr) + paylen);
> +	if (chunklen > SCTP_MAX_CHUNK_LEN)
> +		goto nodata;
>  
>  	/* No need to allocate LL here, as this is only a chunk. */
> -	skb = alloc_skb(SCTP_PAD4(sizeof(*chunk_hdr) + paylen), gfp);
> +	skb = alloc_skb(chunklen, gfp);
>  	if (!skb)
>  		goto nodata;
>  
> -- 
> 1.8.3.1
> 
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

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

* Re: [PATCH net v2] sctp: verify size of a new chunk in _sctp_make_chunk()
@ 2018-02-09 15:02   ` Neil Horman
  0 siblings, 0 replies; 8+ messages in thread
From: Neil Horman @ 2018-02-09 15:02 UTC (permalink / raw)
  To: Alexey Kodanev
  Cc: netdev, Marcelo Ricardo Leitner, Vlad Yasevich, David Miller, linux-sctp

On Fri, Feb 09, 2018 at 05:35:23PM +0300, Alexey Kodanev wrote:
> 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:<NULL>
> ...
> [  597.976970] ------------[ cut here ]------------
> [  598.033408] kernel BUG at net/core/skbuff.c:104!
> [  600.314841] Call Trace:
> [  600.345829]  <IRQ>
> [  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 <alexey.kodanev@oracle.com>
> ---
> 
> v2: account for padding before checking chunklen
> 
>  net/sctp/sm_make_chunk.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 793b05e..d01475f 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -1380,9 +1380,14 @@ static struct sctp_chunk *_sctp_make_chunk(const struct sctp_association *asoc,
>  	struct sctp_chunk *retval;
>  	struct sk_buff *skb;
>  	struct sock *sk;
> +	int chunklen;
> +
> +	chunklen = SCTP_PAD4(sizeof(*chunk_hdr) + paylen);
> +	if (chunklen > SCTP_MAX_CHUNK_LEN)
> +		goto nodata;
>  
>  	/* No need to allocate LL here, as this is only a chunk. */
> -	skb = alloc_skb(SCTP_PAD4(sizeof(*chunk_hdr) + paylen), gfp);
> +	skb = alloc_skb(chunklen, gfp);
>  	if (!skb)
>  		goto nodata;
>  
> -- 
> 1.8.3.1
> 
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>


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

* Re: [PATCH net v2] sctp: verify size of a new chunk in _sctp_make_chunk()
  2018-02-09 14:35 ` Alexey Kodanev
@ 2018-02-09 19:32   ` David Miller
  -1 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2018-02-09 19:32 UTC (permalink / raw)
  To: alexey.kodanev; +Cc: netdev, marcelo.leitner, nhorman, vyasevich, linux-sctp

From: Alexey Kodanev <alexey.kodanev@oracle.com>
Date: Fri,  9 Feb 2018 17:35:23 +0300

> 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:
 ...
> 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 <alexey.kodanev@oracle.com>

Applied and queued up for -stable, thanks.

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

* Re: [PATCH net v2] sctp: verify size of a new chunk in _sctp_make_chunk()
@ 2018-02-09 19:32   ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2018-02-09 19:32 UTC (permalink / raw)
  To: alexey.kodanev; +Cc: netdev, marcelo.leitner, nhorman, vyasevich, linux-sctp

From: Alexey Kodanev <alexey.kodanev@oracle.com>
Date: Fri,  9 Feb 2018 17:35:23 +0300

> 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:
 ...
> 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 <alexey.kodanev@oracle.com>

Applied and queued up for -stable, thanks.

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

end of thread, other threads:[~2018-02-09 19:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-09 14:35 [PATCH net v2] sctp: verify size of a new chunk in _sctp_make_chunk() Alexey Kodanev
2018-02-09 14:35 ` Alexey Kodanev
2018-02-09 14:40 ` Marcelo Ricardo Leitner
2018-02-09 14:40   ` Marcelo Ricardo Leitner
2018-02-09 15:02 ` Neil Horman
2018-02-09 15:02   ` Neil Horman
2018-02-09 19:32 ` David Miller
2018-02-09 19:32   ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.