Date: Sat, 28 Apr 2012 13:16:01 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Matt Miller <matt@matthewjmiller.net> Cc: net@freebsd.org Subject: Re: Alloc Error Handling in lib/libc/rpc/svc.c Message-ID: <20120428101601.GG2358@deviant.kiev.zoral.com.ua> In-Reply-To: <CAFc6gu-VNpk966JwRwUEiMvQFDds-ryErS5DLk3f-Oh6Qf-_5g@mail.gmail.com> References: <CAFc6gu-VNpk966JwRwUEiMvQFDds-ryErS5DLk3f-Oh6Qf-_5g@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
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;
[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (FreeBSD)
iEYEARECAAYFAk+bw2EACgkQC3+MBN1Mb4gOcwCfS1NhTjDDL85COhQhWbYU4Cj9
4YoAniCN5ua921mGMaxnUjualTTlXKFq
=7ObQ
-----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120428101601.GG2358>