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.
>
> The fix is to simply move the variables outside the nested
> 'if' block. To keep valgrind quiet we also zero-initialize
> the 'control' buffer.
>
> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> ---
> io/channel-socket.c | 7 ++-
> tests/test-io-channel-socket.c | 98
> ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 101 insertions(+), 4 deletions(-)
>
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 == G_N_ELEMENTS(fdsend));
> + /* Each recvd FD should be different from sent FD */
> + for (i = 0; i < nfdrecv; i++) {
> + g_assert_cmpint(fdrecv[i], !=, testfd);
> + }
Here, you blindly dereference fdrecv[]...
> + unlink(TEST_FILE);
> + close(testfd);
> + if (fdrecv != NULL) {
...so this if() is dead, and you can just always do the cleanup.
That's minor, so:
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
