After discussion with ad@: it appears that KERNEL_LOCK also protects

the driver output path (that is, ifp->if_output()).  In the case of
entry through the socket code, we are fine, because pru_usrreq takes
KERNEL_LOCK.  However, there are a few other ways to cause output
which require protection:

	1) direct calls to tcp_output() in tcp_input()
	2) fast-forwarding code (ip_flow) -- protected elsewise
	   against itself by the softnet lock.
	3) *Possibly* the ARP code.  I have currently persuaded
	   myself that it is safe because of how it's called.
	4) Possibly the ICMP code.

This change addresses #1 and #2.
This commit is contained in:
tls 2010-04-01 00:24:41 +00:00
parent d6c268fede
commit 994b02bdbe
2 changed files with 16 additions and 4 deletions

View File

@ -1,4 +1,4 @@
/* $NetBSD: ip_flow.c,v 1.58 2009/03/15 22:16:09 cegger Exp $ */ /* $NetBSD: ip_flow.c,v 1.59 2010/04/01 00:24:41 tls Exp $ */
/*- /*-
* Copyright (c) 1998 The NetBSD Foundation, Inc. * Copyright (c) 1998 The NetBSD Foundation, Inc.
@ -30,7 +30,7 @@
*/ */
#include <sys/cdefs.h> #include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: ip_flow.c,v 1.58 2009/03/15 22:16:09 cegger Exp $"); __KERNEL_RCSID(0, "$NetBSD: ip_flow.c,v 1.59 2010/04/01 00:24:41 tls Exp $");
#include <sys/param.h> #include <sys/param.h>
#include <sys/systm.h> #include <sys/systm.h>
@ -301,12 +301,14 @@ ipflow_fastforward(struct mbuf *m)
else else
dst = rtcache_getdst(&ipf->ipf_ro); dst = rtcache_getdst(&ipf->ipf_ro);
KERNEL_LOCK(1, NULL);
if ((error = (*rt->rt_ifp->if_output)(rt->rt_ifp, m, dst, rt)) != 0) { if ((error = (*rt->rt_ifp->if_output)(rt->rt_ifp, m, dst, rt)) != 0) {
if (error == ENOBUFS) if (error == ENOBUFS)
ipf->ipf_dropped++; ipf->ipf_dropped++;
else else
ipf->ipf_errors++; ipf->ipf_errors++;
} }
KERNEL_UNLOCK_ONE(NULL);
return 1; return 1;
} }

View File

@ -1,4 +1,4 @@
/* $NetBSD: tcp_input.c,v 1.300 2010/01/26 18:09:07 pooka Exp $ */ /* $NetBSD: tcp_input.c,v 1.301 2010/04/01 00:24:41 tls Exp $ */
/* /*
* Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project. * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@ -145,7 +145,7 @@
*/ */
#include <sys/cdefs.h> #include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: tcp_input.c,v 1.300 2010/01/26 18:09:07 pooka Exp $"); __KERNEL_RCSID(0, "$NetBSD: tcp_input.c,v 1.301 2010/04/01 00:24:41 tls Exp $");
#include "opt_inet.h" #include "opt_inet.h"
#include "opt_ipsec.h" #include "opt_ipsec.h"
@ -1787,7 +1787,9 @@ after_listen:
sowwakeup(so); sowwakeup(so);
if (so->so_snd.sb_cc) if (so->so_snd.sb_cc)
KERNEL_LOCK(1, NULL);
(void) tcp_output(tp); (void) tcp_output(tp);
KERNEL_UNLOCK_ONE(NULL);
if (tcp_saveti) if (tcp_saveti)
m_freem(tcp_saveti); m_freem(tcp_saveti);
return; return;
@ -1883,7 +1885,9 @@ after_listen:
sorwakeup(so); sorwakeup(so);
tcp_setup_ack(tp, th); tcp_setup_ack(tp, th);
if (tp->t_flags & TF_ACKNOW) if (tp->t_flags & TF_ACKNOW)
KERNEL_LOCK(1, NULL);
(void) tcp_output(tp); (void) tcp_output(tp);
KERNEL_UNLOCK_ONE(NULL);
if (tcp_saveti) if (tcp_saveti)
m_freem(tcp_saveti); m_freem(tcp_saveti);
return; return;
@ -2369,7 +2373,9 @@ after_listen:
goto drop; goto drop;
} else if (tp->t_dupacks > tcprexmtthresh) { } else if (tp->t_dupacks > tcprexmtthresh) {
tp->snd_cwnd += tp->t_segsz; tp->snd_cwnd += tp->t_segsz;
KERNEL_LOCK(1, NULL);
(void) tcp_output(tp); (void) tcp_output(tp);
KERNEL_UNLOCK_ONE(NULL);
goto drop; goto drop;
} }
} else { } else {
@ -2730,7 +2736,9 @@ dodata: /* XXX */
* Return any desired output. * Return any desired output.
*/ */
if (needoutput || (tp->t_flags & TF_ACKNOW)) { if (needoutput || (tp->t_flags & TF_ACKNOW)) {
KERNEL_LOCK(1, NULL);
(void) tcp_output(tp); (void) tcp_output(tp);
KERNEL_UNLOCK_ONE(NULL);
} }
if (tcp_saveti) if (tcp_saveti)
m_freem(tcp_saveti); m_freem(tcp_saveti);
@ -2767,7 +2775,9 @@ dropafterack_ratelim:
dropafterack2: dropafterack2:
m_freem(m); m_freem(m);
tp->t_flags |= TF_ACKNOW; tp->t_flags |= TF_ACKNOW;
KERNEL_LOCK(1, NULL);
(void) tcp_output(tp); (void) tcp_output(tp);
KERNEL_UNLOCK_ONE(NULL);
if (tcp_saveti) if (tcp_saveti)
m_freem(tcp_saveti); m_freem(tcp_saveti);
return; return;