virtio-win / kvm-guest-drivers-windows

Windows paravirtualized drivers for QEMU\KVM
https://www.linux-kvm.org/page/WindowsGuestDrivers
BSD 3-Clause "New" or "Revised" License
1.92k stars 377 forks source link

Viosock: Fix Socket Reference Management #1003

Closed MartinDrab closed 5 months ago

MartinDrab commented 7 months ago

The Socket driver stores socket-related data in memory associated with file objects representing the sockets. It uses WdfObjectReference/WdfObjectDereference to track ownership and protect sockets being in use from deletion. However, these routines do not affect reference count of the underlying FILE_OBJECT structures (served by Object Manager) -- they work only with the reference count of the WDF object wrapped around the FILE_OBJECT structure. Thus, when the underlying FILE_OBJECT is deleted, WdfObjectReference does not protect the socket context from deallocation.

This commit fixes this issue by introducing VioSockReference and VioSockDereference which also work with reference count of the wrapped FILE_OBJECT structures. Also, VioSockClose is replaced with VioSockCleanup – the latter one is invoked when the last handle for a file object (FILE_OBJECT, not WDFFILEOBJECT) gets closed (the former one is invoked when the last reference is gone -- which does not happen if VioSock(De)reference are used). For usermode, this makes no difference since user applications work only with handles, not pointer references to file objects.

MartinDrab commented 7 months ago

I ran into some issues during the second day of testing (all worked fine before that), thus, I suspend this PR until I resolve the problems.

MartinDrab commented 5 months ago

I ran into some issues during the second day of testing (all worked fine before that), thus, I suspend this PR until I resolve the problems.

The issues were not related to this bug, thus, I mark this as ready to merge. I did no changes to the code or commit message, just the branch was moved above the master.

YanVugenfirer commented 5 months ago

Waiting for CI to pass before merging