Switch from spl(9) to mutex(9) and condvar(9).

Fix a defect in the locking of file descriptors as we delegate a
UDP socket from userland to the kernel.  Move sc_fp out of sc_soparm.
Synchronize access to sc_fp by gre_ioctl() and the kernel thread
using a condition variable.  For simplicity's sake, make it the
kernel helper thread's responsibility to close its UDP socket.
This commit is contained in:
dyoung 2007-05-06 02:47:52 +00:00
parent d91019e5e5
commit d9b62cfaed
2 changed files with 129 additions and 103 deletions

View File

@ -1,4 +1,4 @@
/* $NetBSD: if_gre.c,v 1.92 2007/05/02 20:40:23 dyoung Exp $ */
/* $NetBSD: if_gre.c,v 1.93 2007/05/06 02:47:52 dyoung Exp $ */
/*
* Copyright (c) 1998 The NetBSD Foundation, Inc.
@ -48,7 +48,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: if_gre.c,v 1.92 2007/05/02 20:40:23 dyoung Exp $");
__KERNEL_RCSID(0, "$NetBSD: if_gre.c,v 1.93 2007/05/06 02:47:52 dyoung Exp $");
#include "opt_gre.h"
#include "opt_inet.h"
@ -72,6 +72,9 @@ __KERNEL_RCSID(0, "$NetBSD: if_gre.c,v 1.92 2007/05/02 20:40:23 dyoung Exp $");
#include <sys/kauth.h>
#endif
#include <sys/kernel.h>
#include <sys/mutex.h>
#include <sys/condvar.h>
#include <sys/kthread.h>
#include <machine/cpu.h>
@ -138,38 +141,35 @@ static int gre_ioctl(struct ifnet *, u_long, void *);
static int gre_compute_route(struct gre_softc *sc);
static void gre_closef(struct file **, struct lwp *);
static int gre_getsockname(struct socket *, struct mbuf *, struct lwp *);
static int gre_getpeername(struct socket *, struct mbuf *, struct lwp *);
static int gre_getnames(struct socket *, struct lwp *, struct sockaddr_in *,
struct sockaddr_in *);
/* Calling thread must hold sc->sc_mtx. */
static void
gre_stop(volatile int *running)
gre_stop(struct gre_softc *sc)
{
*running = 0;
wakeup(running);
sc->sc_running = 0;
cv_signal(&sc->sc_join_cv);
}
/* Calling thread must hold sc->sc_mtx. */
static void
gre_join(volatile int *running)
gre_join(struct gre_softc *sc)
{
int s;
s = splnet();
while (*running != 0) {
splx(s);
tsleep(running, PSOCK, "grejoin", 0);
s = splnet();
}
splx(s);
while (sc->sc_running != 0)
cv_wait(&sc->sc_join_cv, &sc->sc_mtx);
}
/* Calling thread must hold sc->sc_mtx. */
static void
gre_wakeup(struct gre_softc *sc)
{
GRE_DPRINTF(sc, "%s: enter\n", __func__);
sc->sc_waitchan = 1;
wakeup(&sc->sc_waitchan);
sc->sc_haswork = 1;
cv_signal(&sc->sc_work_cv);
}
static int
@ -179,6 +179,10 @@ gre_clone_create(struct if_clone *ifc, int unit)
sc = malloc(sizeof(struct gre_softc), M_DEVBUF, M_WAITOK);
memset(sc, 0, sizeof(struct gre_softc));
mutex_init(&sc->sc_mtx, MUTEX_DRIVER, IPL_NET);
cv_init(&sc->sc_work_cv, "gre work");
cv_init(&sc->sc_join_cv, "gre join");
cv_init(&sc->sc_soparm_cv, "gre soparm");
snprintf(sc->sc_if.if_xname, sizeof(sc->sc_if.if_xname), "%s%d",
ifc->ifc_name, unit);
@ -208,26 +212,23 @@ gre_clone_create(struct if_clone *ifc, int unit)
static int
gre_clone_destroy(struct ifnet *ifp)
{
int s;
struct gre_softc *sc = ifp->if_softc;
LIST_REMOVE(sc, sc_list);
#if NBPFILTER > 0
bpfdetach(ifp);
#endif
s = splnet();
ifp->if_flags &= ~IFF_UP;
gre_wakeup(sc);
splx(s);
gre_join(&sc->sc_thread);
s = splnet();
if_detach(ifp);
mutex_enter(&sc->sc_mtx);
gre_wakeup(sc);
gre_join(sc);
mutex_exit(&sc->sc_mtx);
rtcache_free(&sc->route);
splx(s);
if (sc->sc_fp != NULL) {
closef(sc->sc_fp, curlwp);
sc->sc_fp = NULL;
}
cv_destroy(&sc->sc_soparm_cv);
cv_destroy(&sc->sc_join_cv);
cv_destroy(&sc->sc_work_cv);
mutex_destroy(&sc->sc_mtx);
free(sc, M_DEVBUF);
return 0;
@ -357,18 +358,19 @@ out:
static void
gre_thread1(struct gre_softc *sc, struct lwp *l)
{
int flags, rc, s;
int flags, rc;
const struct gre_h *gh;
struct ifnet *ifp = &sc->sc_if;
struct mbuf *m;
struct socket *so = NULL;
struct uio uio;
struct gre_soparm sp;
struct file *fp = NULL;
GRE_DPRINTF(sc, "%s: enter\n", __func__);
s = splnet();
mutex_enter(&sc->sc_mtx);
sc->sc_waitchan = 1;
sc->sc_haswork = 1;
memset(&sp, 0, sizeof(sp));
memset(&uio, 0, sizeof(uio));
@ -376,13 +378,11 @@ gre_thread1(struct gre_softc *sc, struct lwp *l)
ifp->if_flags |= IFF_RUNNING;
for (;;) {
while (sc->sc_waitchan == 0) {
splx(s);
while (sc->sc_haswork == 0) {
GRE_DPRINTF(sc, "%s: sleeping\n", __func__);
tsleep(&sc->sc_waitchan, PSOCK, "grewait", 0);
s = splnet();
cv_wait(&sc->sc_work_cv, &sc->sc_mtx);
}
sc->sc_waitchan = 0;
sc->sc_haswork = 0;
GRE_DPRINTF(sc, "%s: awake\n", __func__);
if ((ifp->if_flags & IFF_UP) != IFF_UP) {
GRE_DPRINTF(sc, "%s: not up & running; exiting\n",
@ -394,24 +394,26 @@ gre_thread1(struct gre_softc *sc, struct lwp *l)
break;
}
/* XXX optimize */
if (so == NULL || memcmp(&sp, &sc->sc_soparm, sizeof(sp)) != 0){
if (so == NULL || sc->sc_fp != NULL ||
memcmp(&sp, &sc->sc_soparm, sizeof(sp)) != 0) {
GRE_DPRINTF(sc, "%s: parameters changed\n", __func__);
if (sp.sp_fp != NULL) {
FILE_UNUSE(sp.sp_fp, NULL);
sp.sp_fp = NULL;
if (fp != NULL) {
gre_closef(&fp, curlwp);
so = NULL;
} else if (so != NULL)
gre_sodestroy(&so);
if (sc->sc_fp != NULL) {
so = (struct socket *)sc->sc_fp->f_data;
fp = sc->sc_fp;
sc->sc_fp = NULL;
so = (struct socket *)fp->f_data;
gre_upcall_add(so, sc);
sp = sc->sc_soparm;
FILE_USE(sp.sp_fp);
} else if (gre_socreate1(sc, l, &sp, &so) != 0)
goto out;
}
cv_signal(&sc->sc_soparm_cv);
for (;;) {
flags = MSG_DONTWAIT;
uio.uio_resid = 1000000;
@ -439,7 +441,7 @@ gre_thread1(struct gre_softc *sc, struct lwp *l)
}
gh = mtod(m, const struct gre_h *);
if (gre_input3(sc, m, 0, gh) == 0) {
if (gre_input3(sc, m, 0, gh, 1) == 0) {
GRE_DPRINTF(sc, "%s: dropping unsupported\n",
__func__);
m_freem(m);
@ -462,32 +464,26 @@ gre_thread1(struct gre_softc *sc, struct lwp *l)
GRE_DPRINTF(sc, "%s: so_send failed\n",
__func__);
}
/* Give the software interrupt queues a chance to
* run, or else when I send a ping from gre0 to gre1 on
* the same host, gre0 will not wake for the reply.
*/
splx(s);
s = splnet();
}
if (sp.sp_fp != NULL) {
if (fp != NULL) {
GRE_DPRINTF(sc, "%s: removing upcall\n", __func__);
gre_upcall_remove(so);
FILE_UNUSE(sp.sp_fp, NULL);
sp.sp_fp = NULL;
} else if (so != NULL)
gre_sodestroy(&so);
out:
GRE_DPRINTF(sc, "%s: stopping\n", __func__);
if (fp != NULL)
gre_closef(&fp, curlwp);
if (sc->sc_proto == IPPROTO_UDP)
ifp->if_flags &= ~IFF_RUNNING;
while (!IF_IS_EMPTY(&sc->sc_snd)) {
IF_DEQUEUE(&sc->sc_snd, m);
m_freem(m);
}
gre_stop(&sc->sc_thread);
gre_stop(sc);
/* must not touch sc after this! */
GRE_DPRINTF(sc, "%s: restore ipl\n", __func__);
splx(s);
mutex_exit(&sc->sc_mtx);
}
static void
@ -500,15 +496,16 @@ gre_thread(void *arg)
kthread_exit(0);
}
/* Calling thread must hold sc->sc_mtx. */
int
gre_input3(struct gre_softc *sc, struct mbuf *m, int hlen,
const struct gre_h *gh)
const struct gre_h *gh, int mtx_held)
{
u_int16_t flags;
#if NBPFILTER > 0
u_int32_t af = AF_INET; /* af passed to BPF tap */
#endif
int s, isr;
int isr;
struct ifqueue *ifq;
sc->sc_if.if_ipackets++;
@ -577,7 +574,8 @@ gre_input3(struct gre_softc *sc, struct mbuf *m, int hlen,
m->m_pkthdr.rcvif = &sc->sc_if;
s = splnet(); /* possible */
if (!mtx_held)
mutex_enter(&sc->sc_mtx);
if (IF_QFULL(ifq)) {
IF_DROP(ifq);
m_freem(m);
@ -586,7 +584,8 @@ gre_input3(struct gre_softc *sc, struct mbuf *m, int hlen,
}
/* we need schednetisr since the address family may change */
schednetisr(isr);
splx(s);
if (!mtx_held)
mutex_exit(&sc->sc_mtx);
return 1; /* packet is done, no further processing needed */
}
@ -797,10 +796,7 @@ gre_output(struct ifnet *ifp, struct mbuf *m, const struct sockaddr *dst,
return error;
}
/* gre_kick must be synchronized with network interrupts in order
* to synchronize access to gre_softc members, so call it with
* interrupt priority level set to IPL_NET or greater.
*/
/* Calling thread must hold sc->sc_mtx. */
static int
gre_kick(struct gre_softc *sc)
{
@ -808,11 +804,13 @@ gre_kick(struct gre_softc *sc)
struct ifnet *ifp = &sc->sc_if;
if (sc->sc_proto == IPPROTO_UDP && (ifp->if_flags & IFF_UP) == IFF_UP &&
!sc->sc_thread) {
sc->sc_thread = 1;
!sc->sc_running) {
sc->sc_running = 1;
mutex_exit(&sc->sc_mtx);
rc = kthread_create1(gre_thread, sc, NULL, ifp->if_xname);
mutex_enter(&sc->sc_mtx);
if (rc != 0)
gre_stop(&sc->sc_thread);
gre_stop(sc);
return rc;
} else {
gre_wakeup(sc);
@ -820,29 +818,28 @@ gre_kick(struct gre_softc *sc)
}
}
/* Calling thread must hold sc->sc_mtx. */
static int
gre_getname(struct socket *so, int req, struct mbuf *nam, struct lwp *l)
{
int s, error;
s = splsoftnet();
error = (*so->so_proto->pr_usrreq)(so, req, NULL, nam, NULL, l);
splx(s);
return error;
return (*so->so_proto->pr_usrreq)(so, req, NULL, nam, NULL, l);
}
/* Calling thread must hold sc->sc_mtx. */
static int
gre_getsockname(struct socket *so, struct mbuf *nam, struct lwp *l)
{
return gre_getname(so, PRU_SOCKADDR, nam, l);
}
/* Calling thread must hold sc->sc_mtx. */
static int
gre_getpeername(struct socket *so, struct mbuf *nam, struct lwp *l)
{
return gre_getname(so, PRU_PEERADDR, nam, l);
}
/* Calling thread must hold sc->sc_mtx. */
static int
gre_getnames(struct socket *so, struct lwp *l, struct sockaddr_in *src,
struct sockaddr_in *dst)
@ -877,11 +874,22 @@ out:
return rc;
}
static void
gre_closef(struct file **fpp, struct lwp *l)
{
struct file *fp = *fpp;
simple_lock(&fp->f_slock);
FILE_USE(fp);
closef(fp, l);
*fpp = NULL;
}
static int
gre_ioctl(struct ifnet *ifp, u_long cmd, void *data)
{
u_char oproto;
struct file *fp, *ofp;
struct file *fp;
struct socket *so;
struct sockaddr_in dst, src;
struct proc *p = curproc; /* XXX */
@ -889,7 +897,6 @@ gre_ioctl(struct ifnet *ifp, u_long cmd, void *data)
struct ifreq *ifr = (struct ifreq *)data;
struct if_laddrreq *lifr = (struct if_laddrreq *)data;
struct gre_softc *sc = ifp->if_softc;
int s;
struct sockaddr_in si;
struct sockaddr *sa = NULL;
int error = 0;
@ -913,7 +920,7 @@ gre_ioctl(struct ifnet *ifp, u_long cmd, void *data)
break;
}
s = splnet();
mutex_enter(&sc->sc_mtx);
switch (cmd) {
case SIOCSIFADDR:
ifp->if_flags |= IFF_UP;
@ -1020,10 +1027,6 @@ gre_ioctl(struct ifnet *ifp, u_long cmd, void *data)
if (sc->sc_proto == IPPROTO_UDP ||
(sc->g_src.s_addr != INADDR_ANY &&
sc->g_dst.s_addr != INADDR_ANY)) {
if (sc->sc_fp != NULL) {
closef(sc->sc_fp, l);
sc->sc_fp = NULL;
}
rtcache_free(&sc->route);
if (sc->sc_proto == IPPROTO_UDP)
error = gre_kick(sc);
@ -1050,18 +1053,19 @@ gre_ioctl(struct ifnet *ifp, u_long cmd, void *data)
ifr->ifr_addr = *sa;
break;
case GREDSOCK:
if (sc->sc_proto != IPPROTO_UDP)
return EINVAL;
if (sc->sc_fp != NULL) {
closef(sc->sc_fp, l);
sc->sc_fp = NULL;
error = gre_kick(sc);
if (sc->sc_proto != IPPROTO_UDP) {
error = EINVAL;
break;
}
ifp->if_flags &= ~IFF_UP;
gre_wakeup(sc);
break;
case GRESSOCK:
if (sc->sc_proto != IPPROTO_UDP)
return EINVAL;
/* getsock() will FILE_USE() the descriptor for us */
if (sc->sc_proto != IPPROTO_UDP) {
error = EINVAL;
break;
}
/* getsock() will FILE_USE() and unlock the descriptor for us */
if ((error = getsock(p->p_fd, (int)ifr->ifr_value, &fp)) != 0)
break;
so = (struct socket *)fp->f_data;
@ -1076,21 +1080,37 @@ gre_ioctl(struct ifnet *ifp, u_long cmd, void *data)
break;
}
fp->f_count++;
/* Increase reference count. Now that our reference
* to the file descriptor is counted, this thread
* can release our "use" of the descriptor, but it
* will not be destroyed by some other thread's
* action. This thread needs to release its use,
* too, because one and only one thread can have
* use of the descriptor at once. The kernel thread
* will pick up the use if it needs it.
*/
ofp = sc->sc_fp;
sc->sc_fp = fp;
if ((error = gre_kick(sc)) != 0) {
closef(fp, l);
sc->sc_fp = ofp;
fp->f_count++;
FILE_UNUSE(fp, NULL);
while (sc->sc_fp != NULL && error == 0) {
error = cv_timedwait_sig(&sc->sc_soparm_cv, &sc->sc_mtx,
MAX(1, hz / 2));
}
if (error == 0) {
sc->sc_fp = fp;
ifp->if_flags |= IFF_UP;
}
if (error != 0 || (error = gre_kick(sc)) != 0) {
gre_closef(&fp, l);
break;
}
/* fp does not any longer belong to this thread. */
sc->g_src = src.sin_addr;
sc->g_srcport = src.sin_port;
sc->g_dst = dst.sin_addr;
sc->g_dstport = dst.sin_port;
if (ofp != NULL)
closef(ofp, l);
break;
case SIOCSLIFPHYADDR:
if (lifr->addr.ss_family != AF_INET ||
@ -1136,7 +1156,7 @@ gre_ioctl(struct ifnet *ifp, u_long cmd, void *data)
error = EINVAL;
break;
}
splx(s);
mutex_exit(&sc->sc_mtx);
return error;
}

View File

@ -1,4 +1,4 @@
/* $NetBSD: if_gre.h,v 1.21 2007/03/21 01:56:05 dyoung Exp $ */
/* $NetBSD: if_gre.h,v 1.22 2007/05/06 02:47:52 dyoung Exp $ */
/*
* Copyright (c) 1998 The NetBSD Foundation, Inc.
@ -40,6 +40,8 @@
#define _NET_IF_GRE_H_
#include <sys/queue.h>
#include <sys/mutex.h>
#include <sys/condvar.h>
#ifdef _KERNEL
struct gre_soparm {
@ -47,15 +49,19 @@ struct gre_soparm {
struct in_addr sp_dst; /* destination address of gre packets */
in_port_t sp_srcport; /* source port of gre packets */
in_port_t sp_dstport; /* destination port of gre packets */
struct file *sp_fp;
};
struct gre_softc {
struct ifnet sc_if;
volatile int sc_waitchan;
volatile int sc_thread;
kmutex_t sc_mtx;
kcondvar_t sc_soparm_cv;
kcondvar_t sc_join_cv;
kcondvar_t sc_work_cv;
int sc_haswork;
int sc_running;
struct ifqueue sc_snd;
struct gre_soparm sc_soparm;
struct file *sc_fp;
LIST_ENTRY(gre_softc) sc_list;
struct route route; /* routing entry that determines, where a
encapsulated packet should go */
@ -65,7 +71,6 @@ struct gre_softc {
#define g_srcport sc_soparm.sp_srcport
#define g_dst sc_soparm.sp_dst
#define g_dstport sc_soparm.sp_dstport
#define sc_fp sc_soparm.sp_fp
struct gre_h {
u_int16_t flags; /* GRE flags */
@ -163,7 +168,8 @@ LIST_HEAD(gre_softc_head, gre_softc);
extern struct gre_softc_head gre_softc_list;
u_int16_t gre_in_cksum(u_short *, u_int);
int gre_input3(struct gre_softc *, struct mbuf *, int, const struct gre_h *);
int gre_input3(struct gre_softc *, struct mbuf *, int, const struct gre_h *,
int);
#endif /* _KERNEL */
#endif /* !_NET_IF_GRE_H_ */