From: Eric Dumazet <hidden> Date: 2018-03-08 15:15:22
On 03/08/2018 05:37 AM, Alexander Potapenko wrote:
KMSAN reported a use of uninit memory in vhost_net_buf_unproduce()
while trying to access n->vqs[VHOST_NET_VQ_TX].rx_ring:
==================================================================
Signed-off-by: Alexander Potapenko <glider@google.com>
Please identify bug origin with a Fixes: tag
This will tremendously help stable teams that are struggling with many backports these days.
Thanks.
From: Alexander Potapenko <glider@google.com> Date: 2018-03-08 15:20:51
On Thu, Mar 8, 2018 at 4:15 PM, Eric Dumazet [off-list ref] wrote:
On 03/08/2018 05:37 AM, Alexander Potapenko wrote:
quoted
KMSAN reported a use of uninit memory in vhost_net_buf_unproduce()
while trying to access n->vqs[VHOST_NET_VQ_TX].rx_ring:
==================================================================
Signed-off-by: Alexander Potapenko <glider@google.com>
Please identify bug origin with a Fixes: tag
Fixes: 5990a30510ed1 ("tun/tap: use ptr_ring instead of skb_array")
The above patch introduced rx_ring, but the problem existed before.
This will tremendously help stable teams that are struggling with many
backports these days.
Thanks.
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
From: Eric Dumazet <hidden> Date: 2018-03-08 15:45:21
On 03/08/2018 07:20 AM, Alexander Potapenko wrote:
On Thu, Mar 8, 2018 at 4:15 PM, Eric Dumazet [off-list ref] wrote:
quoted
On 03/08/2018 05:37 AM, Alexander Potapenko wrote:
quoted
KMSAN reported a use of uninit memory in vhost_net_buf_unproduce()
while trying to access n->vqs[VHOST_NET_VQ_TX].rx_ring:
==================================================================
Signed-off-by: Alexander Potapenko <glider@google.com>
Please identify bug origin with a Fixes: tag
Fixes: 5990a30510ed1 ("tun/tap: use ptr_ring instead of skb_array")
Please send a V2 with this added tag. patchwork does not recognize it yet.
David Miller has also a lot on his plate, please everybody be gentle with both stable teams and maintainers.
Thanks.
The above patch introduced rx_ring, but the problem existed before.
quoted
This will tremendously help stable teams that are struggling with many
backports these days.
Thanks.
From: Alexander Potapenko <glider@google.com> Date: 2018-03-08 15:50:31
On Thu, Mar 8, 2018 at 4:45 PM, Eric Dumazet [off-list ref] wrote:
On 03/08/2018 07:20 AM, Alexander Potapenko wrote:
quoted
On Thu, Mar 8, 2018 at 4:15 PM, Eric Dumazet [off-list ref]
wrote:
quoted
On 03/08/2018 05:37 AM, Alexander Potapenko wrote:
quoted
KMSAN reported a use of uninit memory in vhost_net_buf_unproduce()
while trying to access n->vqs[VHOST_NET_VQ_TX].rx_ring:
==================================================================
Signed-off-by: Alexander Potapenko <glider@google.com>
Please identify bug origin with a Fixes: tag
Fixes: 5990a30510ed1 ("tun/tap: use ptr_ring instead of skb_array")
Please send a V2 with this added tag. patchwork does not recognize it yet.
Ok, will do. Thanks for reminding about the tag!
David Miller has also a lot on his plate, please everybody be gentle with
both stable teams and maintainers.
Thanks.
quoted
The above patch introduced rx_ring, but the problem existed before.
quoted
This will tremendously help stable teams that are struggling with many
backports these days.
Thanks.
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
I suspect that's not sufficient. rx ring is tied to the tap device.
I think we need to drop it every time we drop the device.
Unfortunately I've no idea where is the device dropped. Are you
referring to vhost_net_vq_reset()?
I can fix that part if needed, but won't be able to validate it with KMSAN.
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
I suspect that's not sufficient. rx ring is tied to the tap device.
I think we need to drop it every time we drop the device.
Unfortunately I've no idea where is the device dropped. Are you
referring to vhost_net_vq_reset()?
I can fix that part if needed, but won't be able to validate it with KMSAN.
I see several issues. For example in vhost_net_set_backend
if there's a value then rx ring will point to the
ring of the wrong socket.
Something like the below might help but we really need
documentation of when is rx_ring valid. Is it only valid
when private-data is valid? If yes need to make sure
we reset it with private_data.
Also I see __skb_array_destroy_skb used with ptr_ring which
seems suspicious: how do we know the entries are skbs?
Patch below is on top of yours, and
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
But I really would like Jason to look and come up with a
patch to address all these issues.
---
I suspect that's not sufficient. rx ring is tied to the tap device.
I think we need to drop it every time we drop the device.
Unfortunately I've no idea where is the device dropped. Are you
referring to vhost_net_vq_reset()?
I can fix that part if needed, but won't be able to validate it with KMSAN.
I see several issues. For example in vhost_net_set_backend
if there's a value then rx ring will point to the
ring of the wrong socket.
Something like the below might help but we really need
documentation of when is rx_ring valid. Is it only valid
when private-data is valid?
I think so, we need keep rx_ring synced with private_data.
If yes need to make sure
we reset it with private_data.
Also I see __skb_array_destroy_skb used with ptr_ring which
seems suspicious: how do we know the entries are skbs?
Good catch, will post an independent patch to fix this.
quoted hunk
Patch below is on top of yours, and
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
But I really would like Jason to look and come up with a
patch to address all these issues.
---
I suspect that's not sufficient. rx ring is tied to the tap device.
I think we need to drop it every time we drop the device.
Unfortunately I've no idea where is the device dropped. Are you
referring to vhost_net_vq_reset()?
I can fix that part if needed, but won't be able to validate it with KMSAN.
I see several issues. For example in vhost_net_set_backend
if there's a value then rx ring will point to the
ring of the wrong socket.
Something like the below might help but we really need
documentation of when is rx_ring valid. Is it only valid
when private-data is valid?
I think so, we need keep rx_ring synced with private_data.
quoted
If yes need to make sure
we reset it with private_data.
Also I see __skb_array_destroy_skb used with ptr_ring which
seems suspicious: how do we know the entries are skbs?
Good catch, will post an independent patch to fix this.
quoted
Patch below is on top of yours, and
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
But I really would like Jason to look and come up with a
patch to address all these issues.
---
I suspect that's not sufficient. rx ring is tied to the tap device.
I think we need to drop it every time we drop the device.
Unfortunately I've no idea where is the device dropped. Are you
referring to vhost_net_vq_reset()?
I can fix that part if needed, but won't be able to validate it with KMSAN.
I see several issues. For example in vhost_net_set_backend
if there's a value then rx ring will point to the
ring of the wrong socket.
Something like the below might help but we really need
documentation of when is rx_ring valid. Is it only valid
when private-data is valid?
I think so, we need keep rx_ring synced with private_data.
quoted
If yes need to make sure
we reset it with private_data.
Also I see __skb_array_destroy_skb used with ptr_ring which
seems suspicious: how do we know the entries are skbs?
Good catch, will post an independent patch to fix this.
quoted
Patch below is on top of yours, and
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
But I really would like Jason to look and come up with a
patch to address all these issues.
---
From: Jason Wang <hidden> Date: 2018-03-09 06:33:25
On 2018年03月08日 23:50, Alexander Potapenko wrote:
On Thu, Mar 8, 2018 at 4:45 PM, Eric Dumazet[off-list ref] wrote:
quoted
On 03/08/2018 07:20 AM, Alexander Potapenko wrote:
quoted
On Thu, Mar 8, 2018 at 4:15 PM, Eric Dumazet[off-list ref]
wrote:
quoted
On 03/08/2018 05:37 AM, Alexander Potapenko wrote:
quoted
KMSAN reported a use of uninit memory in vhost_net_buf_unproduce()
while trying to access n->vqs[VHOST_NET_VQ_TX].rx_ring:
==================================================================
Signed-off-by: Alexander Potapenko<glider@google.com>
Please identify bug origin with a Fixes: tag
Fixes: 5990a30510ed1 ("tun/tap: use ptr_ring instead of skb_array")
Please send a V2 with this added tag. patchwork does not recognize it yet.
Ok, will do. Thanks for reminding about the tag!
The commit should be c67df11f6e480 ("vhost_net: try batch dequing from skb array").
Let me squash this patch into my series.
Thanks