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
--I5DvuL84SVjYoYOv
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
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.
>=20
> - No NULL checks after mem_alloc()'s:
>=20
> SVCXPRT *
> svc_xprt_alloc()
> {
> SVCXPRT *xprt;
> SVCXPRT_EXT *ext;
>=20
> xprt =3D mem_alloc(sizeof(SVCXPRT));
> memset(xprt, 0, sizeof(SVCXPRT));
> ext =3D mem_alloc(sizeof(SVCXPRT_EXT));
> memset(ext, 0, sizeof(SVCXPRT_EXT));
> xprt->xp_p3 =3D ext;
> ext->xp_auth.svc_ah_ops =3D &svc_auth_null_ops;
>=20
> return (xprt);
> }
>=20
> - No lock release if mem_alloc() returns NULL:
>=20
> void
> xprt_register(xprt)
> SVCXPRT *xprt;
> {
> int sock;
>=20
> assert(xprt !=3D NULL);
>=20
> sock =3D xprt->xp_fd;
>=20
> rwlock_wrlock(&svc_fd_lock);
> if (__svc_xports =3D=3D NULL) {
> __svc_xports =3D (SVCXPRT **)
> mem_alloc(FD_SETSIZE * sizeof(SVCXPRT *));
> if (__svc_xports =3D=3D NULL)
> return;
> memset(__svc_xports, '\0', FD_SETSIZE * sizeof(SVCXPRT *));
> }
> if (sock < FD_SETSIZE) {
> __svc_xports[sock] =3D xprt;
> FD_SET(sock, &svc_fdset);
> svc_maxfd =3D 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 =3D=3D NULL) {
__svc_xports =3D (SVCXPRT **)
mem_alloc(FD_SETSIZE * sizeof(SVCXPRT *));
- if (__svc_xports =3D=3D NULL)
+ if (__svc_xports =3D=3D 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;
=20
xprt =3D mem_alloc(sizeof(SVCXPRT));
+ if (xprt =3D=3D NULL)
+ return (NULL);
memset(xprt, 0, sizeof(SVCXPRT));
ext =3D mem_alloc(sizeof(SVCXPRT_EXT));
+ if (ext =3D=3D NULL) {
+ mem_free(xprt, sizeof(SVCXPRT));
+ return (NULL);
+ }
memset(ext, 0, sizeof(SVCXPRT_EXT));
xprt->xp_p3 =3D ext;
ext->xp_auth.svc_ah_ops =3D &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 =3D=3D NULL)
+ if (__rpc_rawcombuf =3D=3D NULL) {
__rpc_rawcombuf =3D calloc(UDPMSGSIZE, sizeof (char));
+ if (__rpc_rawcombuf =3D=3D NULL) {
+ free(srp);
+ mutex_unlock(&svcraw_lock);
+ return (NULL);
+ }
+ }
srp->raw_buf =3D __rpc_rawcombuf; /* Share it with the client */
srp->server =3D svc_xprt_alloc();
+ if (srp->server =3D=3D NULL) {
+ free(__rpc_rawcombuf);
+ free(srp);
+ mutex_unlock(&svcraw_lock);
+ return (NULL);
+ }
svc_raw_private =3D srp;
}
srp->server->xp_fd =3D FD_SETSIZE;
--I5DvuL84SVjYoYOv
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (FreeBSD)
iEYEARECAAYFAk+bw2EACgkQC3+MBN1Mb4gOcwCfS1NhTjDDL85COhQhWbYU4Cj9
4YoAniCN5ua921mGMaxnUjualTTlXKFq
=7ObQ
-----END PGP SIGNATURE-----
--I5DvuL84SVjYoYOv--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120428101601.GG2358>