On 2020/7/24 下午11:34, Cornelia Huck wrote:
On Fri, 24 Jul 2020 11:17:57 -0400
"Michael S. Tsirkin"<m...@redhat.com> wrote:
On Fri, Jul 24, 2020 at 04:56:27PM +0200, Cornelia Huck wrote:
On Fri, 24 Jul 2020 09:30:58 -0400
"Michael S. Tsirkin"<m...@redhat.com> wrote:
On Fri, Jul 24, 2020 at 03:27:18PM +0200, Cornelia Huck wrote:
When I start qemu with a second virtio-net-ccw device (i.e. adding
-device virtio-net-ccw in addition to the autogenerated device), I get
a segfault. gdb points to
#0 0x000055d6ab52681d in virtio_net_get_config (vdev=<optimized out>,
config=0x55d6ad9e3f80 "RT") at
/home/cohuck/git/qemu/hw/net/virtio-net.c:146
146 if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
(backtrace doesn't go further)
The core was incomplete, but running under gdb directly shows that it
is just a bog-standard config space access (first for that device).
The cause of the crash is that nc->peer is not set... no idea how that
can happen, not that familiar with that part of QEMU. (Should the code
check, or is that really something that should not happen?)
What I don't understand is why it is set correctly for the first,
autogenerated virtio-net-ccw device, but not for the second one, and
why virtio-net-pci doesn't show these problems. The only difference
between -ccw and -pci that comes to my mind here is that config space
accesses for ccw are done via an asynchronous operation, so timing
might be different.
Hopefully Jason has an idea. Could you post a full command line
please? Do you need a working guest to trigger this? Does this trigger
on an x86 host?
Yes, it does trigger with tcg-on-x86 as well. I've been using
s390x-softmmu/qemu-system-s390x -M s390-ccw-virtio,accel=tcg -cpu qemu,zpci=on
-m 1024 -nographic -device virtio-scsi-ccw,id=scsi0,devno=fe.0.0001
-drive file=/path/to/image,format=qcow2,if=none,id=drive-scsi0-0-0-0
-device
scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1
-device virtio-net-ccw
It seems it needs the guest actually doing something with the nics; I
cannot reproduce the crash if I use the old advent calendar moon buggy
image and just add a virtio-net-ccw device.
(I don't think it's a problem with my local build, as I see the problem
both on my laptop and on an LPAR.)
It looks to me we forget the check the existence of peer.
Please try the attached patch to see if it works.
Thanks
>From f6959056dcc65cbdc256c4af2b1a0eaee784c15f Mon Sep 17 00:00:00 2001
From: Jason Wang <jasow...@redhat.com>
Date: Sat, 25 Jul 2020 08:13:17 +0800
Subject: [PATCH] virtio-net: check the existence of peer before accesing its
config
We try to get config from peer unconditionally which may lead NULL
pointer dereference. Add a check before trying to access the config.
Fixes: 108a64818e69b ("vhost-vdpa: introduce vhost-vdpa backend")
Signed-off-by: Jason Wang <jasow...@redhat.com>
---
hw/net/virtio-net.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 4895af1cbe..935b9ef5c7 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -125,6 +125,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
{
VirtIONet *n = VIRTIO_NET(vdev);
struct virtio_net_config netcfg;
+ NetClientState *nc = qemu_get_queue(n->nic);
int ret = 0;
memset(&netcfg, 0 , sizeof(struct virtio_net_config));
@@ -142,13 +143,12 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
VIRTIO_NET_RSS_SUPPORTED_HASHES);
memcpy(config, &netcfg, n->config_size);
- NetClientState *nc = qemu_get_queue(n->nic);
- if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+ if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
- n->config_size);
- if (ret != -1) {
- memcpy(config, &netcfg, n->config_size);
- }
+ n->config_size);
+ if (ret != -1) {
+ memcpy(config, &netcfg, n->config_size);
+ }
}
}
@@ -156,6 +156,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
{
VirtIONet *n = VIRTIO_NET(vdev);
struct virtio_net_config netcfg = {};
+ NetClientState *nc = qemu_get_queue(n->nic);
memcpy(&netcfg, config, n->config_size);
@@ -166,11 +167,10 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
}
- NetClientState *nc = qemu_get_queue(n->nic);
- if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
- vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
- 0, n->config_size,
- VHOST_SET_CONFIG_TYPE_MASTER);
+ if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+ vhost_net_set_config(get_vhost_net(nc->peer),
+ (uint8_t *)&netcfg, 0, n->config_size,
+ VHOST_SET_CONFIG_TYPE_MASTER);
}
}
--
2.20.1
