svn commit: r189029 - stable/7/sys/net

[Robert Watson]

On Wed, 25 Feb 2009, Robert Watson wrote:

> Author: rwatson
> Date: Wed Feb 25 11:18:18 2009
> New Revision: 189029
> URL: http://svn.freebsd.org/changeset/base/189029
>
> Log:
>  Correct a deadlock and a rtentry leak in rt_check():
>
>  - In the event that a gateway route has to be looked up, drop the lock
>    on 'rt' before reacquiring it 'rt0' in order to avoid deadlock.
>
>  - In the event the original route has evaporated or is no longer up
>    after the gateway route lookup, call RTFREE() on the gateway route
>    before retrying.
>
>  This is a potential errata candidate patch.

Just to clarify preemptively: rt_check() isn't present in 8.x due to the link 
layer routing rewrite, so this was committed directly to 7.x.

Robert N M Watson
Computer Laboratory
University of Cambridge

>
>  PR:		kern/130652
>  Submitted by:	Dmitrij Tejblum <tejblum at yandex-team.ru>
>  Reviewed by:	bz
>  Tested by:	Pete French <petefrench at ticketswitch.com>
>
> Modified:
>  stable/7/sys/net/route.c
>
> Modified: stable/7/sys/net/route.c
> ==============================================================================
> --- stable/7/sys/net/route.c	Wed Feb 25 11:13:13 2009	(r189028)
> +++ stable/7/sys/net/route.c	Wed Feb 25 11:18:18 2009	(r189029)
> @@ -1650,27 +1650,34 @@ retry:
> 				return (ENETUNREACH);
> 			}
> 			/*
> -			 * Relock it and lose the added reference.
> -			 * All sorts of things could have happenned while we
> -			 * had no lock on it, so check for them.
> +			 * Relock it and lose the added reference.  All sorts
> +			 * of things could have happenned while we had no
> +			 * lock on it, so check for them.  rt need to be
> +			 * unlocked to avoid possible deadlock.
> 			 */
> +			RT_UNLOCK(rt);
> 			RT_RELOCK(rt0);
> -			if (rt0 == NULL || ((rt0->rt_flags & RTF_UP) == 0))
> +			if (rt0 == NULL || ((rt0->rt_flags & RTF_UP) == 0)) {
> 				/* Ru-roh.. what we had is no longer any good */
> +				RTFREE(rt);
> 				goto retry;
> +			}
> 			/*
> 			 * While we were away, someone replaced the gateway.
> 			 * Since a reference count is involved we can't just
> 			 * overwrite it.
> 			 */
> 			if (rt0->rt_gwroute) {
> -				if (rt0->rt_gwroute != rt) {
> -					RTFREE_LOCKED(rt);
> -					goto retry;
> -				}
> +				if (rt0->rt_gwroute != rt)
> +					RTFREE(rt);
> 			} else {
> 				rt0->rt_gwroute = rt;
> 			}
> +			/*
> +			 * Since rt was not locked, we need recheck that
> +			 * it still may be used (e.g. up)
> +			 */
> +			goto retry;
> 		}
> 		RT_LOCK_ASSERT(rt);
> 		RT_UNLOCK(rt0);
>