|
|
a23edb1 |
From: Daniel Borkmann <dborkman@redhat.com>
|
|
|
a23edb1 |
Date: Thu, 9 Oct 2014 22:55:32 +0200
|
|
|
a23edb1 |
Subject: [PATCH] net: sctp: fix panic on duplicate ASCONF chunks
|
|
|
a23edb1 |
|
|
|
a23edb1 |
Upstream commit b69040d8e39f20d5215a03502a8e8b4c6ab78395 CVE-2014-3687
|
|
|
a23edb1 |
|
|
|
a23edb1 |
When receiving a e.g. semi-good formed connection scan in the
|
|
|
a23edb1 |
form of ...
|
|
|
a23edb1 |
|
|
|
a23edb1 |
-------------- INIT[ASCONF; ASCONF_ACK] ------------->
|
|
|
a23edb1 |
<----------- INIT-ACK[ASCONF; ASCONF_ACK] ------------
|
|
|
a23edb1 |
-------------------- COOKIE-ECHO -------------------->
|
|
|
a23edb1 |
<-------------------- COOKIE-ACK ---------------------
|
|
|
a23edb1 |
---------------- ASCONF_a; ASCONF_b ----------------->
|
|
|
a23edb1 |
|
|
|
a23edb1 |
... where ASCONF_a equals ASCONF_b chunk (at least both serials
|
|
|
a23edb1 |
need to be equal), we panic an SCTP server!
|
|
|
a23edb1 |
|
|
|
a23edb1 |
The problem is that good-formed ASCONF chunks that we reply with
|
|
|
a23edb1 |
ASCONF_ACK chunks are cached per serial. Thus, when we receive a
|
|
|
a23edb1 |
same ASCONF chunk twice (e.g. through a lost ASCONF_ACK), we do
|
|
|
a23edb1 |
not need to process them again on the server side (that was the
|
|
|
a23edb1 |
idea, also proposed in the RFC). Instead, we know it was cached
|
|
|
a23edb1 |
and we just resend the cached chunk instead. So far, so good.
|
|
|
a23edb1 |
|
|
|
a23edb1 |
Where things get nasty is in SCTP's side effect interpreter, that
|
|
|
a23edb1 |
is, sctp_cmd_interpreter():
|
|
|
a23edb1 |
|
|
|
a23edb1 |
While incoming ASCONF_a (chunk = event_arg) is being marked
|
|
|
a23edb1 |
!end_of_packet and !singleton, and we have an association context,
|
|
|
a23edb1 |
we do not flush the outqueue the first time after processing the
|
|
|
a23edb1 |
ASCONF_ACK singleton chunk via SCTP_CMD_REPLY. Instead, we keep it
|
|
|
a23edb1 |
queued up, although we set local_cork to 1. Commit 2e3216cd54b1
|
|
|
a23edb1 |
changed the precedence, so that as long as we get bundled, incoming
|
|
|
a23edb1 |
chunks we try possible bundling on outgoing queue as well. Before
|
|
|
a23edb1 |
this commit, we would just flush the output queue.
|
|
|
a23edb1 |
|
|
|
a23edb1 |
Now, while ASCONF_a's ASCONF_ACK sits in the corked outq, we
|
|
|
a23edb1 |
continue to process the same ASCONF_b chunk from the packet. As
|
|
|
a23edb1 |
we have cached the previous ASCONF_ACK, we find it, grab it and
|
|
|
a23edb1 |
do another SCTP_CMD_REPLY command on it. So, effectively, we rip
|
|
|
a23edb1 |
the chunk->list pointers and requeue the same ASCONF_ACK chunk
|
|
|
a23edb1 |
another time. Since we process ASCONF_b, it's correctly marked
|
|
|
a23edb1 |
with end_of_packet and we enforce an uncork, and thus flush, thus
|
|
|
a23edb1 |
crashing the kernel.
|
|
|
a23edb1 |
|
|
|
a23edb1 |
Fix it by testing if the ASCONF_ACK is currently pending and if
|
|
|
a23edb1 |
that is the case, do not requeue it. When flushing the output
|
|
|
a23edb1 |
queue we may relink the chunk for preparing an outgoing packet,
|
|
|
a23edb1 |
but eventually unlink it when it's copied into the skb right
|
|
|
a23edb1 |
before transmission.
|
|
|
a23edb1 |
|
|
|
a23edb1 |
Joint work with Vlad Yasevich.
|
|
|
a23edb1 |
|
|
|
a23edb1 |
Fixes: 2e3216cd54b1 ("sctp: Follow security requirement of responding with 1 packet")
|
|
|
a23edb1 |
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
|
|
|
a23edb1 |
Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>
|
|
|
a23edb1 |
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
|
a23edb1 |
---
|
|
|
a23edb1 |
include/net/sctp/sctp.h | 5 +++++
|
|
|
a23edb1 |
net/sctp/associola.c | 2 ++
|
|
|
a23edb1 |
2 files changed, 7 insertions(+)
|
|
|
a23edb1 |
|
|
|
a23edb1 |
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
|
|
|
a23edb1 |
index 9fbd856e6713..856f01cb51dd 100644
|
|
|
a23edb1 |
--- a/include/net/sctp/sctp.h
|
|
|
a23edb1 |
+++ b/include/net/sctp/sctp.h
|
|
|
a23edb1 |
@@ -426,6 +426,11 @@ static inline void sctp_assoc_pending_pmtu(struct sock *sk, struct sctp_associat
|
|
|
a23edb1 |
asoc->pmtu_pending = 0;
|
|
|
a23edb1 |
}
|
|
|
a23edb1 |
|
|
|
a23edb1 |
+static inline bool sctp_chunk_pending(const struct sctp_chunk *chunk)
|
|
|
a23edb1 |
+{
|
|
|
a23edb1 |
+ return !list_empty(&chunk->list);
|
|
|
a23edb1 |
+}
|
|
|
a23edb1 |
+
|
|
|
a23edb1 |
/* Walk through a list of TLV parameters. Don't trust the
|
|
|
a23edb1 |
* individual parameter lengths and instead depend on
|
|
|
a23edb1 |
* the chunk length to indicate when to stop. Make sure
|
|
|
a23edb1 |
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
|
|
|
a23edb1 |
index a88b8524846e..f791edd64d6c 100644
|
|
|
a23edb1 |
--- a/net/sctp/associola.c
|
|
|
a23edb1 |
+++ b/net/sctp/associola.c
|
|
|
a23edb1 |
@@ -1668,6 +1668,8 @@ struct sctp_chunk *sctp_assoc_lookup_asconf_ack(
|
|
|
a23edb1 |
* ack chunk whose serial number matches that of the request.
|
|
|
a23edb1 |
*/
|
|
|
a23edb1 |
list_for_each_entry(ack, &asoc->asconf_ack_list, transmitted_list) {
|
|
|
a23edb1 |
+ if (sctp_chunk_pending(ack))
|
|
|
a23edb1 |
+ continue;
|
|
|
a23edb1 |
if (ack->subh.addip_hdr->serial == serial) {
|
|
|
a23edb1 |
sctp_chunk_hold(ack);
|
|
|
a23edb1 |
return ack;
|
|
|
a23edb1 |
--
|
|
|
a23edb1 |
1.9.3
|
|
|
a23edb1 |
|