svn commit: r242517 - user/andre/tcp_workqueue/sys/kern

[Andre Oppermann]

On 03.11.2012 18:49, Andre Oppermann wrote:
> Author: andre
> Date: Sat Nov  3 17:49:04 2012
> New Revision: 242517
> URL: http://svn.freebsd.org/changeset/base/242517
>
> Log:
>    Fix a race on listen socket teardown where while draining the
>    accept queues a new socket/connection may be added to the queue
>    due to a race on the ACCEPT_LOCK.
>
>    The submitted patch is slightly changed in comments, teardown
>    and locking order and extended with KASSERT's.
>
>    Submitted by:	Vijay Singh <vijju.singh-at-gmail-dot-com>
>    Found by:	His team.

I'm waiting on Vijay's feedback on my changes and merge to HEAD
when I've received his report.

-- 
Andre

> Modified:
>    user/andre/tcp_workqueue/sys/kern/uipc_socket.c
>
> Modified: user/andre/tcp_workqueue/sys/kern/uipc_socket.c
> ==============================================================================
> --- user/andre/tcp_workqueue/sys/kern/uipc_socket.c	Sat Nov  3 16:06:14 2012	(r242516)
> +++ user/andre/tcp_workqueue/sys/kern/uipc_socket.c	Sat Nov  3 17:49:04 2012	(r242517)
> @@ -555,6 +555,16 @@ sonewconn(struct socket *head, int conns
>   	so->so_snd.sb_flags |= head->so_snd.sb_flags & SB_AUTOSIZE;
>   	so->so_state |= connstatus;
>   	ACCEPT_LOCK();
> +	/*
> +	 * The accept socket may be tearing down but we just
> +	 * won a race on the ACCEPT_LOCK.
> +	 */
> +	if (!(so->so_options & SO_ACCEPTCONN)) {
> +		SOCK_LOCK(so);
> +		so->so_head = NULL;
> +		sofree(so);		/* NB: returns ACCEPT_UNLOCK'ed. */
> +		return (NULL);
> +	}
>   	if (connstatus) {
>   		TAILQ_INSERT_TAIL(&head->so_comp, so, so_list);
>   		so->so_qstate |= SQ_COMP;
> @@ -780,9 +790,14 @@ soclose(struct socket *so)
>   drop:
>   	if (so->so_proto->pr_usrreqs->pru_close != NULL)
>   		(*so->so_proto->pr_usrreqs->pru_close)(so);
> +	ACCEPT_LOCK();
>   	if (so->so_options & SO_ACCEPTCONN) {
>   		struct socket *sp;
> -		ACCEPT_LOCK();
> +		/*
> +		 * Prevent new additions to the accept queues due
> +		 * to ACCEPT_LOCK races while we are draining them.
> +		 */
> +		so->so_options &= ~SO_ACCEPTCONN;
>   		while ((sp = TAILQ_FIRST(&so->so_incomp)) != NULL) {
>   			TAILQ_REMOVE(&so->so_incomp, sp, so_list);
>   			so->so_incqlen--;
> @@ -801,9 +816,11 @@ drop:
>   			soabort(sp);
>   			ACCEPT_LOCK();
>   		}
> -		ACCEPT_UNLOCK();
> +		KASSERT((TAILQ_EMPTY(&so->so_comp)),
> +		    ("%s: so_comp populated", __func__));
> +		KASSERT((TAILQ_EMPTY(&so->so_incomp)),
> +		    ("%s: so_incomp populated", __func__));
>   	}
> -	ACCEPT_LOCK();
>   	SOCK_LOCK(so);
>   	KASSERT((so->so_state & SS_NOFDREF) == 0, ("soclose: NOFDREF"));
>   	so->so_state |= SS_NOFDREF;
>
>