From 530c491c673154c5c935bd339c6d00850d454190 Mon Sep 17 00:00:00 2001 From: Jim Rees Date: Thu, 30 Jan 2003 14:59:00 +0000 Subject: [PATCH] fix-netreceive-memleak-20030130 rx_knet fixes for Darwin and FreeBSD: netreceive: fix memory leak, check return code from soreceive netsend: remove unnecessary mbuf alloc, remove misleading comment all: general cleanup and minor bug fixes Thanks to emoy@apple.com for reporting this bug and testing the fix --- src/rx/DARWIN/rx_knet.c | 133 ++++++++++++++++++------------------------------ src/rx/FBSD/rx_kmutex.h | 2 +- src/rx/FBSD/rx_knet.c | 115 +++++++++++++++++++---------------------- src/rx/OBSD/rx_knet.c | 17 +------ 4 files changed, 106 insertions(+), 161 deletions(-) diff --git a/src/rx/DARWIN/rx_knet.c b/src/rx/DARWIN/rx_knet.c index f02b010..d81cdd0 100644 --- a/src/rx/DARWIN/rx_knet.c +++ b/src/rx/DARWIN/rx_knet.c @@ -14,39 +14,35 @@ RCSID("$Header$"); #include "rx/rx_kcommon.h" -int osi_NetReceive(osi_socket so, struct sockaddr_in *addr, struct iovec *dvec, - int nvecs, int *alength) +int osi_NetReceive(osi_socket so, struct sockaddr_in *addr, struct iovec *dvec, + int nvecs, int *alength) { struct socket *asocket = (struct socket *)so; struct uio u; int i; struct iovec iov[RX_MAXIOVECS]; - struct sockaddr *sa; + struct sockaddr *sa = NULL; int code; int haveGlock = ISAFS_GLOCK(); /*AFS_STATCNT(osi_NetReceive);*/ - if (nvecs > RX_MAXIOVECS) { + if (nvecs > RX_MAXIOVECS) osi_Panic("osi_NetReceive: %d: Too many iovecs.\n", nvecs); - } - for (i = 0 ; i < nvecs ; i++) { - iov[i].iov_base = dvec[i].iov_base; - iov[i].iov_len = dvec[i].iov_len; - } + for (i = 0 ; i < nvecs ; i++) + iov[i] = dvec[i]; - u.uio_iov=&iov[0]; - u.uio_iovcnt=nvecs; - u.uio_offset=0; - u.uio_resid=*alength; - u.uio_segflg=UIO_SYSSPACE; - u.uio_rw=UIO_READ; - u.uio_procp=NULL; + u.uio_iov = &iov[0]; + u.uio_iovcnt = nvecs; + u.uio_offset = 0; + u.uio_resid = *alength; + u.uio_segflg = UIO_SYSSPACE; + u.uio_rw = UIO_READ; + u.uio_procp = NULL; - if (haveGlock) { + if (haveGlock) AFS_GUNLOCK(); - } #if defined(AFS_DARWIN_ENV) && defined(KERNEL_FUNNEL) thread_funnel_switch(KERNEL_FUNNEL, NETWORK_FUNNEL); #endif @@ -54,16 +50,19 @@ int osi_NetReceive(osi_socket so, struct sockaddr_in *addr, struct iovec *dvec, #if defined(AFS_DARWIN_ENV) && defined(KERNEL_FUNNEL) thread_funnel_switch(NETWORK_FUNNEL, KERNEL_FUNNEL); #endif - if (haveGlock) { + if (haveGlock) AFS_GLOCK(); - } - *alength=*alength-u.uio_resid; + + if (code) + return code; + *alength -= u.uio_resid; if (sa) { - if (sa->sa_family == AF_INET) { - if (addr) *addr=*(struct sockaddr_in *)sa; - } else { - printf("Unknown socket family %d in NetReceive\n"); - } + if (sa->sa_family == AF_INET) { + if (addr) + *addr = *(struct sockaddr_in *) sa; + } else + printf("Unknown socket family %d in NetReceive\n", sa->sa_family); + FREE(sa, M_SONAME); } return code; } @@ -71,89 +70,57 @@ int osi_NetReceive(osi_socket so, struct sockaddr_in *addr, struct iovec *dvec, extern int rxk_ListenerPid; void osi_StopListener(void) { - struct proc *p; + struct proc *p; #if defined(AFS_DARWIN_ENV) && defined(KERNEL_FUNNEL) thread_funnel_switch(KERNEL_FUNNEL, NETWORK_FUNNEL); #endif - soclose(rx_socket); + soclose(rx_socket); #if defined(AFS_DARWIN_ENV) && defined(KERNEL_FUNNEL) thread_funnel_switch(NETWORK_FUNNEL, KERNEL_FUNNEL); #endif - p=pfind(rxk_ListenerPid); - if (p) - psignal(p, SIGUSR1); + p=pfind(rxk_ListenerPid); + if (p) + psignal(p, SIGUSR1); } -/* rx_NetSend - send asize bytes at adata from asocket to host at addr. - * - * Now, why do we allocate a new buffer when we could theoretically use the one - * pointed to by adata? Because PRU_SEND returns after queueing the message, - * not after sending it. If the sender changes the data after queueing it, - * we'd see the already-queued data change. One attempt to fix this without - * adding a copy would be to have this function wait until the datagram is - * sent; however this doesn't work well. In particular, if a host is down, and - * an ARP fails to that host, this packet will be queued until the ARP request - * comes back, which could be hours later. We can't block in this routine that - * long, since it prevents RPC timeouts from happening. - */ -/* XXX In the brave new world, steal the data bufs out of the rx_packet iovec, - * and just queue those. XXX - */ - - int osi_NetSend(osi_socket asocket, struct sockaddr_in *addr, struct iovec *dvec, int nvecs, afs_int32 alength, int istack) { register afs_int32 code; - int s; - int len; int i; struct iovec iov[RX_MAXIOVECS]; - char *tdata; struct uio u; - struct mbuf *nam; int haveGlock = ISAFS_GLOCK(); AFS_STATCNT(osi_NetSend); - if (nvecs > RX_MAXIOVECS) { + if (nvecs > RX_MAXIOVECS) osi_Panic("osi_NetSend: %d: Too many iovecs.\n", nvecs); - } - - for (i = 0 ; i < nvecs ; i++) { - iov[i].iov_base = dvec[i].iov_base; - iov[i].iov_len = dvec[i].iov_len; - } - - u.uio_iov=&iov[0]; - u.uio_iovcnt=nvecs; - u.uio_offset=0; - u.uio_resid=alength; - u.uio_segflg=UIO_SYSSPACE; - u.uio_rw=UIO_WRITE; - u.uio_procp=NULL; - if (haveGlock) { - AFS_GUNLOCK(); - } + + for (i = 0 ; i < nvecs ; i++) + iov[i] = dvec[i]; + + u.uio_iov = &iov[0]; + u.uio_iovcnt = nvecs; + u.uio_offset = 0; + u.uio_resid = alength; + u.uio_segflg = UIO_SYSSPACE; + u.uio_rw = UIO_WRITE; + u.uio_procp = NULL; + + addr->sin_len = sizeof(struct sockaddr_in); + + if (haveGlock) + AFS_GUNLOCK(); #if defined(AFS_DARWIN_ENV) && defined(KERNEL_FUNNEL) thread_funnel_switch(KERNEL_FUNNEL, NETWORK_FUNNEL); #endif - nam=m_get(M_DONTWAIT, MT_SONAME); - if (nam == NULL) { - code=ENOBUFS; - goto bad; - } - nam->m_len=addr->sin_len=sizeof(struct sockaddr_in); - memcpy(mtod(nam, caddr_t), (caddr_t)addr, addr->sin_len); - code = sosend(asocket, mtod(nam, struct sockaddr *), &u, NULL, NULL, 0); - m_freem(nam); -bad: + code = sosend(asocket, (struct sockaddr *) addr, &u, NULL, NULL, 0); #if defined(AFS_DARWIN_ENV) && defined(KERNEL_FUNNEL) thread_funnel_switch(NETWORK_FUNNEL, KERNEL_FUNNEL); #endif - if (haveGlock) { - AFS_GLOCK(); - } + if (haveGlock) + AFS_GLOCK(); return code; } diff --git a/src/rx/FBSD/rx_kmutex.h b/src/rx/FBSD/rx_kmutex.h index acf9b98..ec9bb74 100644 --- a/src/rx/FBSD/rx_kmutex.h +++ b/src/rx/FBSD/rx_kmutex.h @@ -52,7 +52,7 @@ #define CV_SIGNAL(cv) wakeup_one(cv) #define CV_BROADCAST(cv) wakeup(cv) -#define osi_rxWakeup(cv) wakeup(cv) +/* #define osi_rxWakeup(cv) wakeup(cv) */ typedef int afs_kcondvar_t; #define HEAVY_LOCKS diff --git a/src/rx/FBSD/rx_knet.c b/src/rx/FBSD/rx_knet.c index d4883d7..5aa504e 100644 --- a/src/rx/FBSD/rx_knet.c +++ b/src/rx/FBSD/rx_knet.c @@ -26,62 +26,61 @@ RCSID("$Header$"); #ifdef AFS_FBSD40_ENV +#include #include "rx/rx_kcommon.h" - #ifdef RXK_LISTENER_ENV -int osi_NetReceive(osi_socket so, struct sockaddr_in *addr, struct iovec *dvec, - int nvecs, int *alength) -{ +int osi_NetReceive(osi_socket so, struct sockaddr_in *addr, struct iovec *dvec, + int nvecs, int *alength) +{ struct socket *asocket = (struct socket *)so; struct uio u; int i; struct iovec iov[RX_MAXIOVECS]; - struct sockaddr *sa; + struct sockaddr *sa = NULL; int code; int haveGlock = ISAFS_GLOCK(); /*AFS_STATCNT(osi_NetReceive);*/ - if (nvecs > RX_MAXIOVECS) { + if (nvecs > RX_MAXIOVECS) osi_Panic("osi_NetReceive: %d: Too many iovecs.\n", nvecs); - } - for (i = 0 ; i < nvecs ; i++) { - iov[i].iov_base = dvec[i].iov_base; - iov[i].iov_len = dvec[i].iov_len; - } + for (i = 0 ; i < nvecs ; i++) + iov[i] = dvec[i]; - u.uio_iov=&iov[0]; - u.uio_iovcnt=nvecs; - u.uio_offset=0; - u.uio_resid=*alength; - u.uio_segflg=UIO_SYSSPACE; - u.uio_rw=UIO_READ; - u.uio_procp=NULL; + u.uio_iov = &iov[0]; + u.uio_iovcnt = nvecs; + u.uio_offset = 0; + u.uio_resid = *alength; + u.uio_segflg = UIO_SYSSPACE; + u.uio_rw = UIO_READ; + u.uio_procp = NULL; - if (haveGlock) { + if (haveGlock) AFS_GUNLOCK(); - } code = soreceive(asocket, &sa, &u, NULL, NULL, NULL); -#if KNET_DEBUG + if (haveGlock) + AFS_GLOCK(); + if (code) { +#if KNET_DEBUG if (code == EINVAL) - Debugger("afs NetReceive busted"); + Debugger("afs NetReceive busted"); else - printf("y"); - } + printf("y"); +#else + return code; #endif - if (haveGlock) { - AFS_GLOCK(); } - *alength=*alength-u.uio_resid; + *alength -= u.uio_resid; if (sa) { - if (sa->sa_family == AF_INET) { - if (addr) *addr=*(struct sockaddr_in *)sa; - } else { - printf("Unknown socket family %d in NetReceive\n", sa->sa_family); - } + if (sa->sa_family == AF_INET) { + if (addr) + *addr = *(struct sockaddr_in *) sa; + } else + printf("Unknown socket family %d in NetReceive\n", sa->sa_family); + FREE(sa, M_SONAME); } return code; } @@ -89,65 +88,57 @@ int osi_NetReceive(osi_socket so, struct sockaddr_in *addr, struct iovec *dvec, extern int rxk_ListenerPid; void osi_StopListener(void) { - struct proc *p; + struct proc *p; - soclose(rx_socket); - p=pfind(rxk_ListenerPid); - if (p) - psignal(p, SIGUSR1); + soclose(rx_socket); + p = pfind(rxk_ListenerPid); + if (p) + psignal(p, SIGUSR1); } -int +int osi_NetSend(osi_socket asocket, struct sockaddr_in *addr, struct iovec *dvec, int nvecs, afs_int32 alength, int istack) { register afs_int32 code; - int s; - int len; int i; struct iovec iov[RX_MAXIOVECS]; - char *tdata; struct uio u; int haveGlock = ISAFS_GLOCK(); AFS_STATCNT(osi_NetSend); - if (nvecs > RX_MAXIOVECS) { + if (nvecs > RX_MAXIOVECS) osi_Panic("osi_NetSend: %d: Too many iovecs.\n", nvecs); - } - for (i = 0 ; i < nvecs ; i++) { - iov[i].iov_base = dvec[i].iov_base; - iov[i].iov_len = dvec[i].iov_len; - } + for (i = 0 ; i < nvecs ; i++) + iov[i] = dvec[i]; - u.uio_iov=&iov[0]; - u.uio_iovcnt=nvecs; - u.uio_offset=0; - u.uio_resid=alength; - u.uio_segflg=UIO_SYSSPACE; - u.uio_rw=UIO_WRITE; - u.uio_procp=NULL; + u.uio_iov = &iov[0]; + u.uio_iovcnt = nvecs; + u.uio_offset = 0; + u.uio_resid = alength; + u.uio_segflg = UIO_SYSSPACE; + u.uio_rw = UIO_WRITE; + u.uio_procp = NULL; - addr->sin_len=sizeof(struct sockaddr_in); + addr->sin_len = sizeof(struct sockaddr_in); - if (haveGlock) { + if (haveGlock) AFS_GUNLOCK(); - } #if KNET_DEBUG printf("+"); #endif - code = sosend(asocket, addr, &u, NULL, NULL, 0, curproc); + code = sosend(asocket, (struct sockaddr *) addr, &u, NULL, NULL, 0, curproc); #if KNET_DEBUG if (code) { if (code == EINVAL) - Debugger("afs NetSend busted"); + Debugger("afs NetSend busted"); else - printf("z"); + printf("z"); } #endif - if (haveGlock) { + if (haveGlock) AFS_GLOCK(); - } return code; } #else diff --git a/src/rx/OBSD/rx_knet.c b/src/rx/OBSD/rx_knet.c index 26b226d..50ec710 100644 --- a/src/rx/OBSD/rx_knet.c +++ b/src/rx/OBSD/rx_knet.c @@ -73,23 +73,10 @@ void osi_StopListener(void) psignal(p, SIGUSR1); } -/* rx_NetSend - send asize bytes at adata from asocket to host at addr. - * - * Now, why do we allocate a new buffer when we could theoretically use the one - * pointed to by adata? Because PRU_SEND returns after queueing the message, - * not after sending it. If the sender changes the data after queueing it, - * we'd see the already-queued data change. One attempt to fix this without - * adding a copy would be to have this function wait until the datagram is - * sent; however this doesn't work well. In particular, if a host is down, and - * an ARP fails to that host, this packet will be queued until the ARP request - * comes back, which could be hours later. We can't block in this routine that - * long, since it prevents RPC timeouts from happening. - */ -/* XXX In the brave new world, steal the data bufs out of the rx_packet iovec, - * and just queue those. XXX +/* + * rx_NetSend - send asize bytes at adata from asocket to host at addr. */ - int osi_NetSend(osi_socket asocket, struct sockaddr_in *addr, struct iovec *dvec, int nvecs, afs_int32 alength, int istack) { -- 1.9.4