Re: [PULL 00/63] virtio,pci,pc: features,fixes

[Michael S. Tsirkin]

On Tue, Jul 23, 2024 at 12:18:48PM +0200, Hanna Czenczek wrote:
> 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<mst@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.

drop what specifically?