Re: [PATCH v8 34/50] vhost/net: virtio 1.0 byte swap
On Wed, Jan 07, 2015 at 09:31:05AM +0100, Greg Kurz wrote: > On Tue, 06 Jan 2015 16:55:30 -0700 > Alex Williamson wrote: > > > On Mon, 2014-12-01 at 18:05 +0200, Michael S. Tsirkin wrote: > > > I had to add an explicit tag to suppress compiler warning: > > > gcc isn't smart enough to notice that > > > len is always initialized since function is called with size > 0. > > > > I'm getting a panic inside a guest when this change is applied on the > > host. I identified this patch via bisect and confirmed by reverting it > > from v3.19-rc2. Guest is centos6. Thanks, > > > > Alex > > > > commit 8b38694a2dc8b18374310df50174f1e4376d6824 > > Author: Michael S. Tsirkin > > Date: Fri Oct 24 14:19:48 2014 +0300 > > > > vhost/net: virtio 1.0 byte swap > > > > I had to add an explicit tag to suppress compiler warning: > > gcc isn't smart enough to notice that > > len is always initialized since function is called with size > 0. > > > > Signed-off-by: Michael S. Tsirkin > > Reviewed-by: Cornelia Huck > > > > This chunk looks suspicious: > > - heads[headcount - 1].len += datalen; > + heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen); > > s/len - datalen/len + datalen/ ? Indeed! I just sent a patch fixing this, thanks a lot. > > XML chunk: > > > > > > > > > > > >> function='0x0'/> > > > > > > Panic log: > > > > <1>BUG: unable to handle kernel NULL pointer dereference at 0010 > > <1>IP: [] virtnet_poll+0x4f9/0x910 [virtio_net] > > <4>PGD 1aa2f4067 PUD 1aa2f5067 PMD 0 > > <4>Oops: [#1] SMP > > <4>last sysfs file: > > /sys/devices/pci:00/:00:03.0/virtio0/net/eth9/ifindex > > <4>CPU 0 > > <4>Modules linked in: 8021q garp stp llc ipt_REJECT nf_conntrack_ipv4 > > nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 > > nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 uinput > > microcode snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep > > snd_seq snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc igbvf > > nvidia(P)(U) i2c_core tg3 ptp pps_core virtio_balloon virtio_net > > virtio_console ext4 jbd2 mbcache virtio_blk virtio_pci virtio_ring virtio > > pata_acpi ata_generic ata_piix dm_mirror dm_region_hash dm_log dm_mod [last > > unloaded: speedstep_lib] > > <4> > > <4>Pid: 1374, comm: NetworkManager Tainted: P --- > > 2.6.32-431.23.3.el6.centos.plus.x86_64 #1 QEMU Standard PC (i440FX + PIIX, > > 1996) > > <4>RIP: 0010:[] [] > > virtnet_poll+0x4f9/0x910 [virtio_net] > > <4>RSP: 0018:880028203e48 EFLAGS: 00010246 > > <4>RAX: 8801a3383d00 RBX: 8801a6aaf480 RCX: 8801aa20b6e0 > > <4>RDX: 00c0 RSI: 8801a3383c00 RDI: 8801a3383cc0 > > <4>RBP: 880028203ed8 R08: 009e R09: 8801aa1d800c > > <4>R10: 0218 R11: R12: 8801aa20b6e0 > > <4>R13: R14: R15: > > <4>FS: 7febf114d800() GS:88002820() > > knlGS: > > <4>CS: 0010 DS: ES: CR0: 80050033 > > <4>CR2: 0010 CR3: 0001aa793000 CR4: 06f0 > > <4>DR0: DR1: DR2: > > <4>DR3: DR6: 0ff0 DR7: 0400 > > <4>Process NetworkManager (pid: 1374, threadinfo 8801a74ba000, task > > 8801a8d56040) > > <4>Stack: > > <4> 8801aa1d8000 009e 8801aa20b6e0 8801aa20b718 > > <4> 8801aa20b780 8801aa1d800c 8801a6aaf4b8 8801aa20b020 > > <4> 0080 8801aa20b708 0001 1f5981a830c8 > > <4>Call Trace: > > <4> > > <4> [] net_rx_action+0x103/0x2f0 > > <4> [] __do_softirq+0xc1/0x1e0 > > <4> [] ? call_softirq+0x1c/0x30 > > <4> [] call_softirq+0x1c/0x30 > > <4> > > <4> [] ? do_softirq+0x65/0xa0 > > <4> [] local_bh_enable+0x9a/0xb0 > > <4> [] virtnet_napi_enable+0x4a/0x60 [virtio_net] > > <4> [] virtnet_open+0x4f/0x60 [virtio_net] > > <4> [] dev_open+0xa1/0x100 > > <4> [] dev_change_flags+0xa1/0x1d0 > > <4> [] do_setlink+0x169/0x8b0 > > <4> [] ? rtnl_fill_ifinfo+0x946/0xcb0 > > <4> [] ? nla_parse+0x34/0x110 > > <4> [] rtnl_setlink+0xee/0x130 > > <4> [] rtnetlink_rcv_msg+0x2d7/0x340 > > <4> [] ? socket_has_perm+0x74/0x90 > > <4> [] ? rtnetlink_rcv_msg+0x0/0x340 > > <4> [] netlink_rcv_skb+0xa9/0xd0 > > <4> [] rtnetlink_rcv+0x25/0x40 > > <4> [] netlink_unicast+0x2db/0x320 > > <4> [] netlink_sendmsg+0x2c0/0x3d0 > > <4> [] sock_sendmsg+0x123/0x150 > > <4> [] ? sock_recvmsg+0x133/0x160 > > <4> [] ? autoremove_wake_function+0x0/0x40 > > <4> [] ? lru_cache_add_lru+0x21/0x40 > > <4> [] ? page_add_new_anon_rmap+0x9d/0xf0 > > <4> [] ? handle_pte_fault+0x4af/0xb00 > > <4> [] ? move_addr_to_kernel+0x64/0x70 > > <4> [] __sys_sendmsg+0x406/0x420 > > <4> [] ? __do_page_fault+0x1ec/0x480 > > <4> [] ? sys_sendto+0x139/0x190 > >
Re: [PATCH v8 34/50] vhost/net: virtio 1.0 byte swap
On Tue, 06 Jan 2015 16:55:30 -0700 Alex Williamson wrote: > On Mon, 2014-12-01 at 18:05 +0200, Michael S. Tsirkin wrote: > > I had to add an explicit tag to suppress compiler warning: > > gcc isn't smart enough to notice that > > len is always initialized since function is called with size > 0. > > I'm getting a panic inside a guest when this change is applied on the > host. I identified this patch via bisect and confirmed by reverting it > from v3.19-rc2. Guest is centos6. Thanks, > > Alex > > commit 8b38694a2dc8b18374310df50174f1e4376d6824 > Author: Michael S. Tsirkin > Date: Fri Oct 24 14:19:48 2014 +0300 > > vhost/net: virtio 1.0 byte swap > > I had to add an explicit tag to suppress compiler warning: > gcc isn't smart enough to notice that > len is always initialized since function is called with size > 0. > > Signed-off-by: Michael S. Tsirkin > Reviewed-by: Cornelia Huck > This chunk looks suspicious: - heads[headcount - 1].len += datalen; + heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen); s/len - datalen/len + datalen/ ? > XML chunk: > > > > > >function='0x0'/> > > > Panic log: > > <1>BUG: unable to handle kernel NULL pointer dereference at 0010 > <1>IP: [] virtnet_poll+0x4f9/0x910 [virtio_net] > <4>PGD 1aa2f4067 PUD 1aa2f5067 PMD 0 > <4>Oops: [#1] SMP > <4>last sysfs file: > /sys/devices/pci:00/:00:03.0/virtio0/net/eth9/ifindex > <4>CPU 0 > <4>Modules linked in: 8021q garp stp llc ipt_REJECT nf_conntrack_ipv4 > nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 > nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 uinput > microcode snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_seq > snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc igbvf > nvidia(P)(U) i2c_core tg3 ptp pps_core virtio_balloon virtio_net > virtio_console ext4 jbd2 mbcache virtio_blk virtio_pci virtio_ring virtio > pata_acpi ata_generic ata_piix dm_mirror dm_region_hash dm_log dm_mod [last > unloaded: speedstep_lib] > <4> > <4>Pid: 1374, comm: NetworkManager Tainted: P --- > 2.6.32-431.23.3.el6.centos.plus.x86_64 #1 QEMU Standard PC (i440FX + PIIX, > 1996) > <4>RIP: 0010:[] [] > virtnet_poll+0x4f9/0x910 [virtio_net] > <4>RSP: 0018:880028203e48 EFLAGS: 00010246 > <4>RAX: 8801a3383d00 RBX: 8801a6aaf480 RCX: 8801aa20b6e0 > <4>RDX: 00c0 RSI: 8801a3383c00 RDI: 8801a3383cc0 > <4>RBP: 880028203ed8 R08: 009e R09: 8801aa1d800c > <4>R10: 0218 R11: R12: 8801aa20b6e0 > <4>R13: R14: R15: > <4>FS: 7febf114d800() GS:88002820() > knlGS: > <4>CS: 0010 DS: ES: CR0: 80050033 > <4>CR2: 0010 CR3: 0001aa793000 CR4: 06f0 > <4>DR0: DR1: DR2: > <4>DR3: DR6: 0ff0 DR7: 0400 > <4>Process NetworkManager (pid: 1374, threadinfo 8801a74ba000, task > 8801a8d56040) > <4>Stack: > <4> 8801aa1d8000 009e 8801aa20b6e0 8801aa20b718 > <4> 8801aa20b780 8801aa1d800c 8801a6aaf4b8 8801aa20b020 > <4> 0080 8801aa20b708 0001 1f5981a830c8 > <4>Call Trace: > <4> > <4> [] net_rx_action+0x103/0x2f0 > <4> [] __do_softirq+0xc1/0x1e0 > <4> [] ? call_softirq+0x1c/0x30 > <4> [] call_softirq+0x1c/0x30 > <4> > <4> [] ? do_softirq+0x65/0xa0 > <4> [] local_bh_enable+0x9a/0xb0 > <4> [] virtnet_napi_enable+0x4a/0x60 [virtio_net] > <4> [] virtnet_open+0x4f/0x60 [virtio_net] > <4> [] dev_open+0xa1/0x100 > <4> [] dev_change_flags+0xa1/0x1d0 > <4> [] do_setlink+0x169/0x8b0 > <4> [] ? rtnl_fill_ifinfo+0x946/0xcb0 > <4> [] ? nla_parse+0x34/0x110 > <4> [] rtnl_setlink+0xee/0x130 > <4> [] rtnetlink_rcv_msg+0x2d7/0x340 > <4> [] ? socket_has_perm+0x74/0x90 > <4> [] ? rtnetlink_rcv_msg+0x0/0x340 > <4> [] netlink_rcv_skb+0xa9/0xd0 > <4> [] rtnetlink_rcv+0x25/0x40 > <4> [] netlink_unicast+0x2db/0x320 > <4> [] netlink_sendmsg+0x2c0/0x3d0 > <4> [] sock_sendmsg+0x123/0x150 > <4> [] ? sock_recvmsg+0x133/0x160 > <4> [] ? autoremove_wake_function+0x0/0x40 > <4> [] ? lru_cache_add_lru+0x21/0x40 > <4> [] ? page_add_new_anon_rmap+0x9d/0xf0 > <4> [] ? handle_pte_fault+0x4af/0xb00 > <4> [] ? move_addr_to_kernel+0x64/0x70 > <4> [] __sys_sendmsg+0x406/0x420 > <4> [] ? __do_page_fault+0x1ec/0x480 > <4> [] ? sys_sendto+0x139/0x190 > <4> [] ? kvm_clock_read+0x1c/0x20 > <4> [] sys_sendmsg+0x49/0x90 > <4> [] system_call_fastpath+0x16/0x1b > <4>Code: 83 e0 00 00 00 00 10 00 00 48 03 93 d0 00 00 00 66 83 42 04 01 8b 93 > cc 00 00 00 48 8b b3 d0 00 00 00 80 4c 16 10 20 44 2b 68 0c <4d> 8b 76 10 75 > 89 e9 d1 fd ff ff 0f 1f 40 00 a8 02 74 0d 0f b6 > <1>RIP [] virtnet_poll+0x4f9/0x91
Re: [PATCH v8 34/50] vhost/net: virtio 1.0 byte swap
On Mon, 2014-12-01 at 18:05 +0200, Michael S. Tsirkin wrote: > I had to add an explicit tag to suppress compiler warning: > gcc isn't smart enough to notice that > len is always initialized since function is called with size > 0. I'm getting a panic inside a guest when this change is applied on the host. I identified this patch via bisect and confirmed by reverting it from v3.19-rc2. Guest is centos6. Thanks, Alex commit 8b38694a2dc8b18374310df50174f1e4376d6824 Author: Michael S. Tsirkin Date: Fri Oct 24 14:19:48 2014 +0300 vhost/net: virtio 1.0 byte swap I had to add an explicit tag to suppress compiler warning: gcc isn't smart enough to notice that len is always initialized since function is called with size > 0. Signed-off-by: Michael S. Tsirkin Reviewed-by: Cornelia Huck XML chunk: Panic log: <1>BUG: unable to handle kernel NULL pointer dereference at 0010 <1>IP: [] virtnet_poll+0x4f9/0x910 [virtio_net] <4>PGD 1aa2f4067 PUD 1aa2f5067 PMD 0 <4>Oops: [#1] SMP <4>last sysfs file: /sys/devices/pci:00/:00:03.0/virtio0/net/eth9/ifindex <4>CPU 0 <4>Modules linked in: 8021q garp stp llc ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 uinput microcode snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc igbvf nvidia(P)(U) i2c_core tg3 ptp pps_core virtio_balloon virtio_net virtio_console ext4 jbd2 mbcache virtio_blk virtio_pci virtio_ring virtio pata_acpi ata_generic ata_piix dm_mirror dm_region_hash dm_log dm_mod [last unloaded: speedstep_lib] <4> <4>Pid: 1374, comm: NetworkManager Tainted: P --- 2.6.32-431.23.3.el6.centos.plus.x86_64 #1 QEMU Standard PC (i440FX + PIIX, 1996) <4>RIP: 0010:[] [] virtnet_poll+0x4f9/0x910 [virtio_net] <4>RSP: 0018:880028203e48 EFLAGS: 00010246 <4>RAX: 8801a3383d00 RBX: 8801a6aaf480 RCX: 8801aa20b6e0 <4>RDX: 00c0 RSI: 8801a3383c00 RDI: 8801a3383cc0 <4>RBP: 880028203ed8 R08: 009e R09: 8801aa1d800c <4>R10: 0218 R11: R12: 8801aa20b6e0 <4>R13: R14: R15: <4>FS: 7febf114d800() GS:88002820() knlGS: <4>CS: 0010 DS: ES: CR0: 80050033 <4>CR2: 0010 CR3: 0001aa793000 CR4: 06f0 <4>DR0: DR1: DR2: <4>DR3: DR6: 0ff0 DR7: 0400 <4>Process NetworkManager (pid: 1374, threadinfo 8801a74ba000, task 8801a8d56040) <4>Stack: <4> 8801aa1d8000 009e 8801aa20b6e0 8801aa20b718 <4> 8801aa20b780 8801aa1d800c 8801a6aaf4b8 8801aa20b020 <4> 0080 8801aa20b708 0001 1f5981a830c8 <4>Call Trace: <4> <4> [] net_rx_action+0x103/0x2f0 <4> [] __do_softirq+0xc1/0x1e0 <4> [] ? call_softirq+0x1c/0x30 <4> [] call_softirq+0x1c/0x30 <4> <4> [] ? do_softirq+0x65/0xa0 <4> [] local_bh_enable+0x9a/0xb0 <4> [] virtnet_napi_enable+0x4a/0x60 [virtio_net] <4> [] virtnet_open+0x4f/0x60 [virtio_net] <4> [] dev_open+0xa1/0x100 <4> [] dev_change_flags+0xa1/0x1d0 <4> [] do_setlink+0x169/0x8b0 <4> [] ? rtnl_fill_ifinfo+0x946/0xcb0 <4> [] ? nla_parse+0x34/0x110 <4> [] rtnl_setlink+0xee/0x130 <4> [] rtnetlink_rcv_msg+0x2d7/0x340 <4> [] ? socket_has_perm+0x74/0x90 <4> [] ? rtnetlink_rcv_msg+0x0/0x340 <4> [] netlink_rcv_skb+0xa9/0xd0 <4> [] rtnetlink_rcv+0x25/0x40 <4> [] netlink_unicast+0x2db/0x320 <4> [] netlink_sendmsg+0x2c0/0x3d0 <4> [] sock_sendmsg+0x123/0x150 <4> [] ? sock_recvmsg+0x133/0x160 <4> [] ? autoremove_wake_function+0x0/0x40 <4> [] ? lru_cache_add_lru+0x21/0x40 <4> [] ? page_add_new_anon_rmap+0x9d/0xf0 <4> [] ? handle_pte_fault+0x4af/0xb00 <4> [] ? move_addr_to_kernel+0x64/0x70 <4> [] __sys_sendmsg+0x406/0x420 <4> [] ? __do_page_fault+0x1ec/0x480 <4> [] ? sys_sendto+0x139/0x190 <4> [] ? kvm_clock_read+0x1c/0x20 <4> [] sys_sendmsg+0x49/0x90 <4> [] system_call_fastpath+0x16/0x1b <4>Code: 83 e0 00 00 00 00 10 00 00 48 03 93 d0 00 00 00 66 83 42 04 01 8b 93 cc 00 00 00 48 8b b3 d0 00 00 00 80 4c 16 10 20 44 2b 68 0c <4d> 8b 76 10 75 89 e9 d1 fd ff ff 0f 1f 40 00 a8 02 74 0d 0f b6 <1>RIP [] virtnet_poll+0x4f9/0x910 [virtio_net] <4> RSP <4>CR2: 0010 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 34/50] vhost/net: virtio 1.0 byte swap
I had to add an explicit tag to suppress compiler warning:
gcc isn't smart enough to notice that
len is always initialized since function is called with size > 0.
Signed-off-by: Michael S. Tsirkin
Reviewed-by: Cornelia Huck
---
drivers/vhost/net.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index dce5c58..c218188 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -416,7 +416,7 @@ static void handle_tx(struct vhost_net *net)
struct ubuf_info *ubuf;
ubuf = nvq->ubuf_info + nvq->upend_idx;
- vq->heads[nvq->upend_idx].id = head;
+ vq->heads[nvq->upend_idx].id = cpu_to_vhost32(vq, head);
vq->heads[nvq->upend_idx].len = VHOST_DMA_IN_PROGRESS;
ubuf->callback = vhost_zerocopy_callback;
ubuf->ctx = nvq->ubufs;
@@ -500,6 +500,10 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
int headcount = 0;
unsigned d;
int r, nlogs = 0;
+ /* len is always initialized before use since we are always called with
+* datalen > 0.
+*/
+ u32 uninitialized_var(len);
while (datalen > 0 && headcount < quota) {
if (unlikely(seg >= UIO_MAXIOV)) {
@@ -527,13 +531,14 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
nlogs += *log_num;
log += *log_num;
}
- heads[headcount].id = d;
- heads[headcount].len = iov_length(vq->iov + seg, in);
- datalen -= heads[headcount].len;
+ heads[headcount].id = cpu_to_vhost32(vq, d);
+ len = iov_length(vq->iov + seg, in);
+ heads[headcount].len = cpu_to_vhost32(vq, len);
+ datalen -= len;
++headcount;
seg += in;
}
- heads[headcount - 1].len += datalen;
+ heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);
*iovcount = seg;
if (unlikely(log))
*log_num = nlogs;
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
