On 22.07.24 23:32, Richard Henderson wrote:
On 7/22/24 10:16, Michael S. Tsirkin wrote:
A couple of fixes are outstanding, will merge later.


The following changes since commit a87a7c449e532130d4fa8faa391ff7e1f04ed660:

   Merge tag 'pull-loongarch-20240719' ofhttps://gitlab.com/gaosong/qemu into staging (2024-07-19 16:28:28 +1000)

are available in the Git repository at:

   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to 67d834362c55d6fca6504975bc34755606f17cf2:

   virtio: Always reset vhost devices (2024-07-21 14:45:56 -0400)

----------------------------------------------------------------
virtio,pci,pc: features,fixes

pci: Initial support for SPDM Responders
cxl: Add support for scan media, feature commands, device patrol scrub
     control, DDR5 ECS control, firmware updates
virtio: in-order support
virtio-net: support for SR-IOV emulation (note: known issues on s390,
                                           might get reverted if not fixed)
smbios: memory device size is now configurable per Machine
cpu: architecture agnostic code to support vCPU Hotplug

Fixes, cleanups all over the place.

Signed-off-by: Michael S. Tsirkin<m...@redhat.com>

Fails ubsan testing:

https://gitlab.com/qemu-project/qemu/-/jobs/7397450714

../publish/hw/net/virtio-net.c:3895:18: runtime error: member access within null pointer of type 'struct vhost_net'

Honestly, I saw this piece of code, but concluded it already doesn’t make sense, so I assumed someone™ who wrote this would know why it’s been written this way, and I should rather not touch it.

Specifically, the problem is that get_vhost_net() can return a NULL pointer[1], which is fine, but virtio_net_get_vhost() never checks this.  I assumed this was written with intent (i.e. `(uintptr_t)&net->dev == (uintptr_t)net`, so that NULL remains NULL), because it’s so obvious that get_vhost_net() can happily return NULL under many circumstances, but maybe not.

The same theoretically applies to virtio_crypto_get_vhost(), although I don’t think that can ever be NULL in practice.

I’ll re-send the reset patch in a series with two patches that fix those two functions to check for NULL and explicitly return NULL if necessary.  In the meantime, it probably makes sense to drop it from this pull request.

Hanna

[1] For some reason, it uses integer 0 throughout to signify NULL. That was another reason that put me off touching this.


Reply via email to