Re: [PATCH v4 05/10] vhost-vsock: preserve vhost FD during CPR

[Stefano Garzarella <sgarzare@redhat.com>]

On Fri, Sep 11, 2026 at 07:35:31PM +0300, Andrey Drobyshev wrote:
>On 9/11/26 10:37 AM, Stefano Garzarella wrote:
>> On Wed, Sep 09, 2026 at 07:59:55PM +0300, Andrey Drobyshev wrote:
>>> On 9/7/26 5:55 PM, Stefano Garzarella wrote:
>>>> On Thu, Aug 20, 2026 at 02:39:50PM +0300, Andrey Drobyshev wrote:
>>>>> During CPR (checkpoint-restore) migration the guest keeps running on the
>>>>> same host, so instead of reopening /dev/vhost-vsock on the destination,
>>>>> we should reuse the FD from the source.  The FD is saved in the CPR
>>>>> namespace (hash table) with cpr_save_fd() and then reclaimed on the
>>>>> target via cpr_find_fd().
>>>>>
>>>>> Since the key in CPR hash table is device ID, CPR needs a unique ID.
>>>>
>>>> mm, are we sure the device ID is unique?
>>>>
>>>> I just tried this whitout receiving any error:
>>>> qemu-system-x86_64 -smp 2 -M q35,accel=kvm,memory-backend=vsock0 \
>>>>      -object memory-backend-memfd,id=vsock0,size=512M \
>>>>      -device vhost-vsock-pci,id=vsock0,guest-cid=3
>>>>
>>>> Stefano
>>> You're right, that seems to be the a real issue.  In fact there's even a
>>> UAF bug IIUC, stemming from the fact that we: 1) provide key destroy
>>> function when creating the CPR hash table; 2) use same CprFd for both
>>> key and value.
>>>
>>> That means that when we have several devices supporting CPR with
>>> repeating IDs, we do:
>>>
>>>  g_hash_table_insert(table, fd1, fd1);    // table has fd1 -> fd1
>>>  g_hash_table_insert(table, fd2, fd2);    // table has fd1 -> fd2
>>>
>>> Here fd1 and fd2 have same hash of course. According to GLib docs [1],
>>> value is renewed in this case, but the new key is destroyed and the 
>>> old
>>> one is used.  As a result, fd2 is freed.
>>>
>>> And, mind you, this all happens long before actual CPR, cause the
>>> devices with CPR support usually call cpr_save_fd() somewhere in
>>> .realize().  Thus we get memory corruption right at boot time, plus
>>> subsequent CPR will likely crash.
>>>
>>> In these circumstances I'd personally prefer just asserting on repeating
>>> key somewhere in cpr_fd_hash_insert().  The only downside is that it'll
>>> crash when booting 2 CPR supporting devices having identical IDs, even
>>> if the user isn't going to perform CPR.  I'd say it's acceptable, what
>>> do you think?
>>
>> I'd prefer an error if possible.
>>
>> That said, can we append a prefix to the key (e.g. `vhost-vsock`)?
>> In this way we reduce the chance to have a conflict. Of course the user
>> can still assign `vhost-vsock-something` to the memory backend and
>> `something` to the vhost-vsock device, but yeah xD
>>
>> Thanks,
>> Stefano
>>
>
>If you mean erroring out directly at startup whenever we encounter a
>duplicate CPR key, with a clear error message instead of abort/crash - I
>agree that it's a better solution.

Yep.

> That'll likely require changing the
>CPR FD saving API, but shouldn't be too intrusive.  As for the prefix -
>once we forbid duplicate keys entirely, there's no need for it.

Agree, but not sure if this break some user that re-use the same id for 
devices in different "namespaces" that will boot, but collide when doing 
CPR. That said, no strong opinion here, maybe I'm overthinking.

Stefano