Re: [PATCH V2 5/7] sctp: use limited socket backlog

[Vlad Yasevich <vladislav.yasevich@hp.com>]

Zhu Yi wrote:
> Make sctp adapt to the limited socket backlog change.
> 
> Cc: Vlad Yasevich <vladislav.yasevich@hp.com>
> Cc: Sridhar Samudrala <sri@us.ibm.com>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
>  net/sctp/input.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index c0c973e..20e69c3 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -75,7 +75,7 @@ static struct sctp_association *__sctp_lookup_association(
>  					const union sctp_addr *peer,
>  					struct sctp_transport **pt);
>  
> -static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb);
> +static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb);
>  
>  
>  /* Calculate the SCTP checksum of an SCTP packet.  */
> @@ -265,8 +265,12 @@ int sctp_rcv(struct sk_buff *skb)
>  	}
>  
>  	if (sock_owned_by_user(sk)) {
> +		if (sctp_add_backlog(sk, skb)) {
> +			sctp_bh_unlock_sock(sk);
> +			sctp_chunk_free(chunk);
> +			goto discard_release;
> +		}

I think this will result in a double-free of the skb, because sctp_chunk_free
attempts to free the skb that's been assigned to the chunk.  You can set the skb
to NULL to get around that.

>  		SCTP_INC_STATS_BH(SCTP_MIB_IN_PKT_BACKLOG);
> -		sctp_add_backlog(sk, skb);
>  	} else {
>  		SCTP_INC_STATS_BH(SCTP_MIB_IN_PKT_SOFTIRQ);
>  		sctp_inq_push(&chunk->rcvr->inqueue, chunk);
> @@ -362,7 +366,7 @@ done:
>  	return 0;
>  }
>  
> -static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
> +static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
>  {
>  	struct sctp_chunk *chunk = SCTP_INPUT_CB(skb)->chunk;
>  	struct sctp_ep_common *rcvr = chunk->rcvr;
> @@ -377,7 +381,7 @@ static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
>  	else
>  		BUG();
>  
> -	sk_add_backlog(sk, skb);
> +	return sk_add_backlog_limited(sk, skb);
>  }

You also leak the ref counts here since now it's possible to not add a packet to
 the backlog queue.  That means you'll take refs, but never drop them because
the receive routing will never run.

-vlad
>  
>  /* Handle icmp frag needed error. */