Re: [PATCH net-next 03/13] virtio_ring: packed: harden dma unmap for indirect
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2024-09-12 07:38:22
Also in:
bpf, virtualization
On Thu, Sep 12, 2024 at 02:55:38PM +0800, Xuan Zhuo wrote:
On Wed, 11 Sep 2024 07:28:36 -0400, "Michael S. Tsirkin" [off-list ref] wrote:quoted
As gcc luckily noted: On Tue, Aug 20, 2024 at 03:33:20PM +0800, Xuan Zhuo wrote:quoted
@@ -1617,23 +1617,24 @@ static void detach_buf_packed(struct vring_virtqueue *vq, } if (vq->indirect) { + struct vring_desc_extra *extra; u32 len; /* Free the indirect table, if any, now that it's unmapped. */ - desc = state->indir_desc; - if (!desc)desc is no longer initialized hereWill fix.quoted
quoted
+ extra = state->indir; + if (!extra) return; if (vring_need_unmap_buffer(vq)) { len = vq->packed.desc_extra[id].len; for (i = 0; i < len / sizeof(struct vring_packed_desc); i++) - vring_unmap_desc_packed(vq, &desc[i]); + vring_unmap_extra_packed(vq, &extra[i]); } kfree(desc);but freed herequoted
- state->indir_desc = NULL; + state->indir = NULL; } else if (ctx) { - *ctx = state->indir_desc; + *ctx = state->indir; } }It seems unlikely this was always 0 on all paths with even a small amount of stress, so now I question how this was tested. Besides, do not ignore compiler warnings, and do not tweak code to just make compiler shut up - they are your friend.I agree. Normally I do this by make W=12, but we have too many message, so I missed this. make W=12 drivers/net/virtio_net.o drivers/virtio/virtio_ring.o If not W=12, then I did not get any warning message. How do you get the message quickly? Thanks.
If you stress test this for a long enough time, and with debug enabled, you will see a crash.
quoted
quoted
-- 2.32.0.3.g01195cf9f