From owner-freebsd-net@FreeBSD.ORG Sat Apr 28 10:16:14 2012 Return-Path: Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 1F8E9106566B for ; Sat, 28 Apr 2012 10:16:14 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 949B78FC0A for ; Sat, 28 Apr 2012 10:16:13 +0000 (UTC) Received: from skuns.kiev.zoral.com.ua (localhost [127.0.0.1]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id q3SAG3NT033247; Sat, 28 Apr 2012 13:16:03 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5) with ESMTP id q3SAG3k4092218; Sat, 28 Apr 2012 13:16:03 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5/Submit) id q3SAG1C5092217; Sat, 28 Apr 2012 13:16:01 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 28 Apr 2012 13:16:01 +0300 From: Konstantin Belousov To: Matt Miller Message-ID: <20120428101601.GG2358@deviant.kiev.zoral.com.ua> References: Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="I5DvuL84SVjYoYOv" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.0 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: net@freebsd.org Subject: Re: Alloc Error Handling in lib/libc/rpc/svc.c X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 28 Apr 2012 10:16:14 -0000 --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--