From owner-freebsd-security@FreeBSD.ORG Tue Jan 14 21:25:48 2014 Return-Path: Delivered-To: freebsd-security@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 5D45F4FC for ; Tue, 14 Jan 2014 21:25:48 +0000 (UTC) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 252FD1AA9 for ; Tue, 14 Jan 2014 21:25:47 +0000 (UTC) Received: from compute5.internal (compute5.nyi.mail.srv.osa [10.202.2.45]) by gateway1.nyi.mail.srv.osa (Postfix) with ESMTP id D7C9A20A32 for ; Tue, 14 Jan 2014 16:25:46 -0500 (EST) Received: from frontend1 ([10.202.2.160]) by compute5.internal (MEProxy); Tue, 14 Jan 2014 16:25:46 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=from:content-type:subject:message-id:date :to:mime-version; s=smtpout; bh=fYV1fSbUnZf7iRVASSdov9Zbp6c=; b= id5r8SoFUVft/V3UI9exhTKp/ywNMztzQkDZVu2+xA29bIebIaBN015GwN+vvzMS v3YVeaZ68IguG6ZgARDkW4gJTvqZEemzmmlqB7Sjm/yNUe8pPTUDLf2UNW2HeELg tVzGZ0d/OqSMu1cYt3bk6tVxHc+vF1Ug0OEpZdDlRRY= X-Sasl-enc: 5B47jut35OepzUrYHtZeWU3gRCdIfb0597s4PS5cXexY 1389734746 Received: from mittlerweile.fritz.box (unknown [118.209.236.29]) by mail.messagingengine.com (Postfix) with ESMTPA id 766DFC00E7F for ; Tue, 14 Jan 2014 16:25:44 -0500 (EST) From: Benno Rice Content-Type: multipart/signed; boundary="Apple-Mail=_E11561FB-F251-497A-B92D-42B5EAAD2AF4"; protocol="application/pgp-signature"; micalg=pgp-sha512 Subject: Review of an OpenCrypto patch Message-Id: <74B9B73E-1F47-40F6-9506-E042608B8931@FreeBSD.org> Date: Wed, 15 Jan 2014 08:25:28 +1100 To: freebsd-security@freebsd.org Mime-Version: 1.0 (Mac OS X Mail 7.1 \(1827\)) X-Mailer: Apple Mail (2.1827) X-Mailman-Approved-At: Tue, 14 Jan 2014 21:30:37 +0000 X-BeenThere: freebsd-security@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Security issues \[members-only posting\]" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Jan 2014 21:25:48 -0000 --Apple-Mail=_E11561FB-F251-497A-B92D-42B5EAAD2AF4 Content-Type: multipart/mixed; boundary="Apple-Mail=_13016BD9-3918-4286-A160-1650966F8977" --Apple-Mail=_13016BD9-3918-4286-A160-1650966F8977 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=windows-1252 Hi -security, I work at EMC Isilon and one of our developers has found a race in = opencyrpto and provided the attached patch to address it. The situation as explained to me is that the crypto request queue and = dequeue operate under CRYPTO_Q_LOCK, along with crypto_invoke and thus = crypto processing. Meanwhile crypto_newsession (and thus all driver new = session calls) operate under CRYPTO_DRIVER_LOCK. This leads to a situation where resizing of the swcr_sessions array in = swcr_newsession can interfere with the use of that array in = swcr_process. The attached patch protects the swcr_sessions array with a new rwlock. Could somebody give this a look over and let me know if it=92s = commitable roughly as is or needs some work? Cheers, Benno. --Apple-Mail=_13016BD9-3918-4286-A160-1650966F8977 Content-Disposition: attachment; filename=patch-117508.3 Content-Type: application/octet-stream; name="patch-117508.3" Content-Transfer-Encoding: quoted-printable Index:=20src/sys/opencrypto/cryptosoft.c=0D=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0D=0A---=20= src/sys/opencrypto/cryptosoft.c=09(revision=20377628)=0D=0A+++=20= src/sys/opencrypto/cryptosoft.c=09(working=20copy)=0D=0A@@=20-35,6=20= +35,8=20@@=0D=0A=20#include=20=0D=0A=20#include=20= =0D=0A=20#include=20=0D=0A+#include=20= =20/*=20Isilon=20bug=20119136=20fix=20*/=0D=0A+#include=20= =20/*=20Isilon=20bug=20119136=20fix=20*/=0D=0A=20=0D=0A=20= #include=20=0D=0A=20#include=20= =0D=0A@@=20-54,6=20+56,7=20@@=0D=0A=20static=09int32_t=20= swcr_id;=0D=0A=20static=09struct=20swcr_data=20**swcr_sessions=20=3D=20= NULL;=0D=0A=20static=09u_int32_t=20swcr_sesnum;=0D=0A+static=20=20struct=20= rwlock=20swcr_sessions_lock;=20/*=20Isilon=20bug=20119136=20fix=20*/=0D=0A= =20=0D=0A=20u_int8_t=20hmac_ipad_buffer[HMAC_MAX_BLOCK_LEN];=0D=0A=20= u_int8_t=20hmac_opad_buffer[HMAC_MAX_BLOCK_LEN];=0D=0A@@=20-618,6=20= +621,7=20@@=0D=0A=20=09if=20(sid=20=3D=3D=20NULL=20||=20cri=20=3D=3D=20= NULL)=0D=0A=20=09=09return=20EINVAL;=0D=0A=20=0D=0A+=09= rw_wlock(&swcr_sessions_lock);=20/*=20Isilon=20bug=20119136=20fix=20*/=0D= =0A=20=09if=20(swcr_sessions)=20{=0D=0A=20=09=09for=20(i=20=3D=201;=20i=20= <=20swcr_sesnum;=20i++)=0D=0A=20=09=09=09if=20(swcr_sessions[i]=20=3D=3D=20= NULL)=0D=0A@@=20-640,6=20+644,7=20@@=0D=0A=20=09=09=09=09swcr_sesnum=20=3D= =200;=0D=0A=20=09=09=09else=0D=0A=20=09=09=09=09swcr_sesnum=20/=3D=202;=0D= =0A+=09=09=09rw_wunlock(&swcr_sessions_lock);=20/*=20Isilon=20bug=20= 119136=20fix=20*/=0D=0A=20=09=09=09return=20ENOBUFS;=0D=0A=20=09=09}=0D=0A= =20=0D=0A@@=20-652,8=20+657,8=20@@=0D=0A=20=0D=0A=20=09=09swcr_sessions=20= =3D=20swd;=0D=0A=20=09}=0D=0A-=0D=0A=20=09swd=20=3D=20&swcr_sessions[i];=0D= =0A+=09rw_wunlock(&swcr_sessions_lock);=20/*=20Isilon=20bug=20119136=20= fix=20*/=0D=0A=20=09*sid=20=3D=20i;=0D=0A=20=0D=0A=20=09while=20(cri)=20= {=0D=0A@@=20-700,7=20+705,7=20@@=0D=0A=20=09=09=09}=0D=0A=20=09=09=09= (*swd)->sw_exf=20=3D=20txf;=0D=0A=20=09=09=09break;=0D=0A-=09=0D=0A+=0D=0A= =20=09=09case=20CRYPTO_MD5_HMAC:=0D=0A=20=09=09=09axf=20=3D=20= &auth_hash_hmac_md5;=0D=0A=20=09=09=09goto=20authcommon;=0D=0A@@=20= -823,13=20+828,18=20@@=0D=0A=20=09struct=20comp_algo=20*cxf;=0D=0A=20=09= u_int32_t=20sid=20=3D=20CRYPTO_SESID2LID(tid);=0D=0A=20=0D=0A+=09= rw_rlock(&swcr_sessions_lock);=20/*=20Isilon=20bug=20119136=20fix=20*/=0D= =0A=20=09if=20(sid=20>=20swcr_sesnum=20||=20swcr_sessions=20=3D=3D=20= NULL=20||=0D=0A-=09=20=20=20=20swcr_sessions[sid]=20=3D=3D=20NULL)=0D=0A= +=09=20=20=20=20swcr_sessions[sid]=20=3D=3D=20NULL)=20{=0D=0A+=09=09= rw_runlock(&swcr_sessions_lock);=20/*=20Isilon=20bug=20119136=20fix=20*/=0D= =0A=20=09=09return=20EINVAL;=0D=0A+=09}=0D=0A=20=0D=0A=20=09/*=20= Silently=20accept=20and=20return=20*/=0D=0A-=09if=20(sid=20=3D=3D=200)=0D= =0A+=09if=20(sid=20=3D=3D=200)=20{=0D=0A+=09=09= rw_runlock(&swcr_sessions_lock);=20/*=20Isilon=20bug=20119136=20fix=20*/=0D= =0A=20=09=09return=200;=0D=0A+=20=20=20=20=20=20=20=20}=0D=0A=20=0D=0A=20= =09while=20((swd=20=3D=20swcr_sessions[sid])=20!=3D=20NULL)=20{=0D=0A=20=09= =09swcr_sessions[sid]=20=3D=20swd->sw_next;=0D=0A@@=20-897,6=20+907,7=20= @@=0D=0A=20=0D=0A=20=09=09FREE(swd,=20M_CRYPTO_DATA);=0D=0A=20=09}=0D=0A= +=09rw_runlock(&swcr_sessions_lock);=0D=0A=20=09return=200;=0D=0A=20}=0D=0A= =20=0D=0A@@=20-920,10=20+931,13=20@@=0D=0A=20=09}=0D=0A=20=0D=0A=20=09= lid=20=3D=20crp->crp_sid=20&=200xffffffff;=0D=0A+=09= rw_rlock(&swcr_sessions_lock);=20/*=20Isilon=20bug=20119136=20fix=20*/=0D= =0A=20=09if=20(lid=20>=3D=20swcr_sesnum=20||=20lid=20=3D=3D=200=20||=20= swcr_sessions[lid]=20=3D=3D=20NULL)=20{=0D=0A+=09=09= rw_runlock(&swcr_sessions_lock);=20/*=20Isilon=20bug=20119136=20fix=20*/=0D= =0A=20=09=09crp->crp_etype=20=3D=20ENOENT;=0D=0A=20=09=09goto=20done;=0D=0A= =20=09}=0D=0A+=09rw_runlock(&swcr_sessions_lock);=20/*=20Isilon=20bug=20= 119136=20fix=20*/=0D=0A=20=0D=0A=20=09/*=20Go=20through=20crypto=20= descriptors,=20processing=20as=20we=20go=20*/=0D=0A=20=09for=20(crd=20=3D=20= crp->crp_desc;=20crd;=20crd=20=3D=20crd->crd_next)=20{=0D=0A@@=20-937,10=20= +951,12=20@@=0D=0A=20=09=09=20*=20XXX=20between=20the=20various=20= instances=20of=20an=20algorithm=20(so=20we=20can=0D=0A=20=09=09=20*=20= XXX=20locate=20the=20correct=20crypto=20context).=0D=0A=20=09=09=20*/=0D=0A= +=09=09rw_rlock(&swcr_sessions_lock);=20/*=20Isilon=20bug=20119136=20fix=20= */=0D=0A=20=09=09for=20(sw=20=3D=20swcr_sessions[lid];=0D=0A=20=09=09=20=20= =20=20sw=20&&=20sw->sw_alg=20!=3D=20crd->crd_alg;=0D=0A=20=09=09=20=20=20= =20sw=20=3D=20sw->sw_next)=0D=0A=20=09=09=09;=0D=0A+=09=09= rw_runlock(&swcr_sessions_lock);=20/*=20Isilon=20bug=20119136=20fix=20*/=0D= =0A=20=0D=0A=20=09=09/*=20No=20such=20context=20?=20*/=0D=0A=20=09=09if=20= (sw=20=3D=3D=20NULL)=20{=0D=0A@@=20-1017,6=20+1033,7=20@@=0D=0A=20static=20= int=0D=0A=20swcr_attach(device_t=20dev)=0D=0A=20{=0D=0A+=09= rw_init(&swcr_sessions_lock,=20"swcr_sessions_lock");=20/*=20Isilon=20= bug=20119136=20fix=20*/=0D=0A=20=09memset(hmac_ipad_buffer,=20= HMAC_IPAD_VAL,=20HMAC_MAX_BLOCK_LEN);=0D=0A=20=09= memset(hmac_opad_buffer,=20HMAC_OPAD_VAL,=20HMAC_MAX_BLOCK_LEN);=0D=0A=20= =0D=0A@@=20-1057,8=20+1074,11=20@@=0D=0A=20swcr_detach(device_t=20dev)=0D= =0A=20{=0D=0A=20=09crypto_unregister_all(swcr_id);=0D=0A+=09= rw_wlock(&swcr_sessions_lock);=0D=0A=20=09if=20(swcr_sessions=20!=3D=20= NULL)=0D=0A=20=09=09FREE(swcr_sessions,=20M_CRYPTO_DATA);=0D=0A+=09= rw_wunlock(&swcr_sessions_lock);=0D=0A+=09= rw_destroy(&swcr_sessions_lock);=0D=0A=20}=0D=0A=20=0D=0A=20static=20= device_method_t=20swcr_methods[]=20=3D=20{=0D=0A= --Apple-Mail=_13016BD9-3918-4286-A160-1650966F8977 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=us-ascii --Apple-Mail=_13016BD9-3918-4286-A160-1650966F8977-- --Apple-Mail=_E11561FB-F251-497A-B92D-42B5EAAD2AF4 Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=signature.asc Content-Type: application/pgp-signature; name=signature.asc Content-Description: Message signed with OpenPGP using GPGMail -----BEGIN PGP SIGNATURE----- Comment: GPGTools - http://gpgtools.org iQIcBAEBCgAGBQJS1atIAAoJELja0BJxpLNeHx0P/ies+Dq8AxenoS7nS47pOLPX i1C4AgNMu8lzBLLfDrhdEWSUp5AEmXte2KLIPr46AnMnnyix1UkJ7LYE26CdjEL8 6fsinOHze8XV8woRM/LPPG2piPJrAlxs+4uGJ+t6/M//pfDaRmkT//vdxTdKu3G3 uSWhOGkQ9D8smCyQfPsll1m+EuuU9IY9CfEKV5iCIHEwsJlUlOWHGhDesK1KUe7R XvJesb0pHVmQDnMnDoF0euFJcS2/SJGrzJzk2XLrOLq1XXzhJNooFulkMgQwdkRT uIP0qGxzyfBJ/h/3xxCDI/P72gnceqtaLJHiNDu3Ol3P/mAOfp9mm4V7lcEvfM5P 7SNiq3tlqLdjkUT9v3+qDM5ziPnpzHuOAbIUMFq66P+veicgIHSWT02+bD0POa1K xDZrKEe2HMCS7QJe0OcTusLvHmNdYyKkG9j+m/Q8j7O59PrOHwJiVSOhcHYvsqmT HHRByYTRoH/rAAdhyKoGzO6SfvbnTOgjHuTmJ0KKtoKPETB2mak4HNlbW175WODu 8lZKTuuVbMqT+ut2gNPYjAyUnwFCygSArJBgQRRLIuPoHlTskC6wPDyEeznpaXDU PccNjddPqdX05UDuZSnetVE577lMUxfk0Xassr4Vke/OcGRnp94+2fvAcpHu0RaU bnf2GGo+zM6Qn2tk4jjp =t28X -----END PGP SIGNATURE----- --Apple-Mail=_E11561FB-F251-497A-B92D-42B5EAAD2AF4--