Re: [Qemu-devel] [PATCH v3 0/4] io: Various fixes around QIOChannel Features

[Felipe Franciosi]

> On 29 Sep 2016, at 17:32, Daniel P. Berrange <address@hidden> wrote:
> 
> On Thu, Sep 29, 2016 at 08:52:34AM -0700, Felipe Franciosi wrote:
>> This series include four patches around the utilisation of QIOChannel
>> features. The first patch actually fixes a bug, while the next two
>> makes the test/set of features consistent by using helper functions.
>> The last patch adds a test to verify that the bug has been fixed.
> 
> Thanks, this all looks good, and I've queued it for my
> next pull request.

For clarification, the bug introduced in 74b6ce43 happens as follows:

On instance_finalize(), any socket with _FEATURE_LISTEN should be cleaned up. 
Instead, sockets with _FEATURE_SHUTDOWN end up being cleaned up. All sockets 
with _LISTEN also have _SHUTDOWN, so it luckily works as intended.

However, sockets with _SHUTDOWN that do not have _LISTEN also get cleaned up 
(and they shouldn't). The issue there is that unix sockets trigger a call to 
unlink() with garbage (an uninitialised field in SocketAddress). Since the 
unlink()'s error code is ENOENT, no one notices (or log) the error.

I couldn't work out a way of exploiting this. Random unlink() calls sound 
serious enough, though. Perhaps you should consider Marc's recommendation and 
also pull it to -stable.

Thanks,
Felipe

> 
> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|