On Tue, Dec 22, 2015 at 11:20:30AM -0700, Eric Blake wrote:
> 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!).
Yes, I wasted lots of time trying to find the flaw before
I wrote the test case at which point it was trivial to
find with valgrind :-)
>
> > + 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.
Yep, will fix
> That's minor, so:
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
