Alloc Error Handling in lib/libc/rpc/svc.c

[Konstantin Belousov]

On Fri, Apr 27, 2012 at 05:48:06PM -0400, Matt Miller wrote:
> In an OOM condition, we noticed a couple of mem_alloc handling bugs in
> this file.  Please let me know if a PR should be opened for these.
> 
> - No NULL checks after mem_alloc()'s:
> 
> SVCXPRT *
> svc_xprt_alloc()
> {
> 	SVCXPRT *xprt;
> 	SVCXPRT_EXT *ext;
> 
> 	xprt = mem_alloc(sizeof(SVCXPRT));
> 	memset(xprt, 0, sizeof(SVCXPRT));
> 	ext = mem_alloc(sizeof(SVCXPRT_EXT));
> 	memset(ext, 0, sizeof(SVCXPRT_EXT));
> 	xprt->xp_p3 = ext;
> 	ext->xp_auth.svc_ah_ops = &svc_auth_null_ops;
> 
> 	return (xprt);
> }
> 
> - No lock release if mem_alloc() returns NULL:
> 
> void
> xprt_register(xprt)
> 	SVCXPRT *xprt;
> {
> 	int sock;
> 
> 	assert(xprt != NULL);
> 
> 	sock = xprt->xp_fd;
> 
> 	rwlock_wrlock(&svc_fd_lock);
> 	if (__svc_xports == NULL) {
> 		__svc_xports = (SVCXPRT **)
> 			mem_alloc(FD_SETSIZE * sizeof(SVCXPRT *));
> 		if (__svc_xports == NULL)
> 			return;
> 		memset(__svc_xports, '\0', FD_SETSIZE * sizeof(SVCXPRT *));
> 	}
> 	if (sock < FD_SETSIZE) {
> 		__svc_xports[sock] = xprt;
> 		FD_SET(sock, &svc_fdset);
> 		svc_maxfd = max(svc_maxfd, sock);
> 	}
> 	rwlock_unlock(&svc_fd_lock);
> }

Thank you for the report.
Does the patch below look fine to you ?

diff --git a/lib/libc/rpc/svc.c b/lib/libc/rpc/svc.c
index 282c2be..78a8ae1 100644
--- a/lib/libc/rpc/svc.c
+++ b/lib/libc/rpc/svc.c
@@ -108,8 +108,10 @@ xprt_register(xprt)
 	if (__svc_xports == NULL) {
 		__svc_xports = (SVCXPRT **)
 			mem_alloc(FD_SETSIZE * sizeof(SVCXPRT *));
-		if (__svc_xports == NULL)
+		if (__svc_xports == NULL) {
+			rwlock_unlock(&svc_fd_lock);
 			return;
+		}
 		memset(__svc_xports, '\0', FD_SETSIZE * sizeof(SVCXPRT *));
 	}
 	if (sock < FD_SETSIZE) {
@@ -565,8 +567,14 @@ svc_xprt_alloc()
 	SVCXPRT_EXT *ext;
 
 	xprt = mem_alloc(sizeof(SVCXPRT));
+	if (xprt == NULL)
+		return (NULL);
 	memset(xprt, 0, sizeof(SVCXPRT));
 	ext = mem_alloc(sizeof(SVCXPRT_EXT));
+	if (ext == NULL) {
+		mem_free(xprt, sizeof(SVCXPRT));
+		return (NULL);
+	}
 	memset(ext, 0, sizeof(SVCXPRT_EXT));
 	xprt->xp_p3 = ext;
 	ext->xp_auth.svc_ah_ops = &svc_auth_null_ops;
diff --git a/lib/libc/rpc/svc_raw.c b/lib/libc/rpc/svc_raw.c
index 67bcba1..de95152 100644
--- a/lib/libc/rpc/svc_raw.c
+++ b/lib/libc/rpc/svc_raw.c
@@ -96,10 +96,22 @@ svc_raw_create()
 			mutex_unlock(&svcraw_lock);
 			return (NULL);
 		}
-		if (__rpc_rawcombuf == NULL)
+		if (__rpc_rawcombuf == NULL) {
 			__rpc_rawcombuf = calloc(UDPMSGSIZE, sizeof (char));
+			if (__rpc_rawcombuf == NULL) {
+				free(srp);
+				mutex_unlock(&svcraw_lock);
+				return (NULL);
+			}
+		}
 		srp->raw_buf = __rpc_rawcombuf; /* Share it with the client */
 		srp->server = svc_xprt_alloc();
+		if (srp->server == NULL) {
+			free(__rpc_rawcombuf);
+			free(srp);
+			mutex_unlock(&svcraw_lock);
+			return (NULL);
+		}
 		svc_raw_private = srp;
 	}
 	srp->server->xp_fd = FD_SETSIZE;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-net/attachments/20120428/ee07c54a/attachment.pgp