From: Neil Horman Date: Sat, 6 May 2006 00:02:09 +0000 (-0700) Subject: [PATCH] SCTP: Allow spillover of receive buffer to avoid deadlock. (CVE-2006-2275) X-Git-Url: http://www.kernel.org/git/?p=linux/kernel/git/stable/linux-2.6.16.y.git;a=commitdiff;h=2e2a2cd09dd7b3fbc99a1879a54090fd6db16f0c [PATCH] SCTP: Allow spillover of receive buffer to avoid deadlock. (CVE-2006-2275) This patch fixes a deadlock situation in the receive path by allowing temporary spillover of the receive buffer. - If the chunk we receive has a tsn that immediately follows the ctsn, accept it even if we run out of receive buffer space and renege data with higher TSNs. - Once we accept one chunk in a packet, accept all the remaining chunks even if we run out of receive buffer space. Signed-off-by: Neil Horman Acked-by: Mark Butler Acked-by: Vlad Yasevich Signed-off-by: Sridhar Samudrala Signed-off-by: David S. Miller Signed-off-by: Chris Wright --- --- a/include/net/sctp/structs.h +++ b/include/net/sctp/structs.h @@ -702,6 +702,7 @@ struct sctp_chunk { __u8 tsn_gap_acked; /* Is this chunk acked by a GAP ACK? */ __s8 fast_retransmit; /* Is this chunk fast retransmitted? */ __u8 tsn_missing_report; /* Data chunk missing counter. */ + __u8 data_accepted; /* At least 1 chunk in this packet accepted */ }; void sctp_chunk_hold(struct sctp_chunk *); --- a/net/sctp/inqueue.c +++ b/net/sctp/inqueue.c @@ -149,6 +149,7 @@ struct sctp_chunk *sctp_inq_pop(struct s /* This is the first chunk in the packet. */ chunk->singleton = 1; ch = (sctp_chunkhdr_t *) chunk->skb->data; + chunk->data_accepted = 0; } chunk->chunk_hdr = ch; --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -5154,7 +5154,9 @@ static int sctp_eat_data(const struct sc int tmp; __u32 tsn; int account_value; + struct sctp_tsnmap *map = (struct sctp_tsnmap *)&asoc->peer.tsn_map; struct sock *sk = asoc->base.sk; + int rcvbuf_over = 0; data_hdr = chunk->subh.data_hdr = (sctp_datahdr_t *)chunk->skb->data; skb_pull(chunk->skb, sizeof(sctp_datahdr_t)); @@ -5165,10 +5167,16 @@ static int sctp_eat_data(const struct sc /* ASSERT: Now skb->data is really the user data. */ /* - * if we are established, and we have used up our receive - * buffer memory, drop the frame + * If we are established, and we have used up our receive buffer + * memory, think about droping the frame. + * Note that we have an opportunity to improve performance here. + * If we accept one chunk from an skbuff, we have to keep all the + * memory of that skbuff around until the chunk is read into user + * space. Therefore, once we accept 1 chunk we may as well accept all + * remaining chunks in the skbuff. The data_accepted flag helps us do + * that. */ - if (asoc->state == SCTP_STATE_ESTABLISHED) { + if ((asoc->state == SCTP_STATE_ESTABLISHED) && (!chunk->data_accepted)) { /* * If the receive buffer policy is 1, then each * association can allocate up to sk_rcvbuf bytes @@ -5179,9 +5187,25 @@ static int sctp_eat_data(const struct sc account_value = atomic_read(&asoc->rmem_alloc); else account_value = atomic_read(&sk->sk_rmem_alloc); - - if (account_value > sk->sk_rcvbuf) - return SCTP_IERROR_IGNORE_TSN; + if (account_value > sk->sk_rcvbuf) { + /* + * We need to make forward progress, even when we are + * under memory pressure, so we always allow the + * next tsn after the ctsn ack point to be accepted. + * This lets us avoid deadlocks in which we have to + * drop frames that would otherwise let us drain the + * receive queue. + */ + if ((sctp_tsnmap_get_ctsn(map) + 1) != tsn) + return SCTP_IERROR_IGNORE_TSN; + + /* + * We're going to accept the frame but we should renege + * to make space for it. This will send us down that + * path later in this function. + */ + rcvbuf_over = 1; + } } /* Process ECN based congestion. @@ -5229,6 +5253,7 @@ static int sctp_eat_data(const struct sc datalen -= sizeof(sctp_data_chunk_t); deliver = SCTP_CMD_CHUNK_ULP; + chunk->data_accepted = 1; /* Think about partial delivery. */ if ((datalen >= asoc->rwnd) && (!asoc->ulpq.pd_mode)) { @@ -5245,7 +5270,8 @@ static int sctp_eat_data(const struct sc * large spill over. */ if (!asoc->rwnd || asoc->rwnd_over || - (datalen > asoc->rwnd + asoc->frag_point)) { + (datalen > asoc->rwnd + asoc->frag_point) || + rcvbuf_over) { /* If this is the next TSN, consider reneging to make * room. Note: Playing nice with a confused sender. A @@ -5253,8 +5279,8 @@ static int sctp_eat_data(const struct sc * space and in the future we may want to detect and * do more drastic reneging. */ - if (sctp_tsnmap_has_gap(&asoc->peer.tsn_map) && - (sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map) + 1) == tsn) { + if (sctp_tsnmap_has_gap(map) && + (sctp_tsnmap_get_ctsn(map) + 1) == tsn) { SCTP_DEBUG_PRINTK("Reneging for tsn:%u\n", tsn); deliver = SCTP_CMD_RENEGE; } else {