From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57519) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBRY3-0004Mt-0B for qemu-devel@nongnu.org; Tue, 22 Dec 2015 13:20:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aBRXz-0006TS-OI for qemu-devel@nongnu.org; Tue, 22 Dec 2015 13:20:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44722) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBRXz-0006TO-8i for qemu-devel@nongnu.org; Tue, 22 Dec 2015 13:20:31 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id CA6ADE708C for ; Tue, 22 Dec 2015 18:20:30 +0000 (UTC) References: <1450715016-18230-1-git-send-email-berrange@redhat.com> <1450715016-18230-3-git-send-email-berrange@redhat.com> From: Eric Blake Message-ID: <5679946E.2060700@redhat.com> Date: Tue, 22 Dec 2015 11:20:30 -0700 MIME-Version: 1.0 In-Reply-To: <1450715016-18230-3-git-send-email-berrange@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="iUO11jjR4PMJkBCBBU85eCvg1R2x6FFOl" Subject: Re: [Qemu-devel] [PATCH 2/2] io: fix stack allocation when sending of file descriptors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --iUO11jjR4PMJkBCBBU85eCvg1R2x6FFOl Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 12/21/2015 09:23 AM, Daniel P. Berrange wrote: > When sending file descriptors over a socket, we have to > allocate a data buffer to hold the FDs in the scmsghdr. > Unfortunately we allocated the buffer on the stack inside > an if () {} block, but called sendmsg() outside the block. > So the stack bytes holding the FDs were liable to be > overwritten with other data. By luck this was not a problem > when sending 1 FD, but if sending 2 or more then it would > fail. >=20 > The fix is to simply move the variables outside the nested > 'if' block. To keep valgrind quiet we also zero-initialize > the 'control' buffer. >=20 > Signed-off-by: Daniel P. Berrange > --- > io/channel-socket.c | 7 ++- > tests/test-io-channel-socket.c | 98 ++++++++++++++++++++++++++++++++++= ++++++++ > 2 files changed, 101 insertions(+), 4 deletions(-) >=20 The fix itself is obvious from the commit message; the bulk of this patch is the testsuite addition (which is a GOOD thing - thanks!). > + qio_channel_readv_full(dst, > + iorecv, > + G_N_ELEMENTS(iorecv), > + &fdrecv, > + &nfdrecv, > + &error_abort); > + > + g_assert(nfdrecv =3D=3D G_N_ELEMENTS(fdsend)); > + /* Each recvd FD should be different from sent FD */ > + for (i =3D 0; i < nfdrecv; i++) { > + g_assert_cmpint(fdrecv[i], !=3D, testfd); > + } Here, you blindly dereference fdrecv[]... > + unlink(TEST_FILE); > + close(testfd); > + if (fdrecv !=3D NULL) { =2E..so this if() is dead, and you can just always do the cleanup. That's minor, so: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --iUO11jjR4PMJkBCBBU85eCvg1R2x6FFOl Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWeZRuAAoJEKeha0olJ0NqYNsIAJtACXBeW358G32kmAb7zsHa i9yo1AtCvpfW1puSGc6SFXJC+zk46xNu1Zt0+5ftv6Kc1Bdt0KxA+ZBxRY1PQ4lG j2GnlMxmObizDr2K1KQF98ghXOkow4wWFvCDKQ8S97pd2e+NU0JBzbc1rBnKCXm+ /4qPoO/hqtbcc27W5sqc/HO8I6RKF5szjelh5E0A0LFW08hZcsmTlWDkjJNUsHYL DBQpLyDsx6+tP7V1bpWb4yhbcHYao3Mv3y2u7vnIhJZqzWp0FshfBzOd9i5l6X9q WzZT9h+BvuWRP5PcP36HvcMiO6ljsx7W3xc6tekGMTukE44LDH1ud8aS5SWaPmU= =N2s5 -----END PGP SIGNATURE----- --iUO11jjR4PMJkBCBBU85eCvg1R2x6FFOl--