在 2022/7/4 10:13, Xuan Zhuo 写道:
On Fri, 1 Jul 2022 17:27:48 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
在 2022/6/29 14:56, Xuan Zhuo 写道:
virtio ring packed supports resize.
Only after the new vring is successfully allocated based on the new num,
we will release the old vring. In any case, an error is returned,
indicating that the vring still points to the old vring.
In the case of an error, re-initialize(by virtqueue_reinit_packed()) the
virtqueue to ensure that the vring can be used.
Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
---
drivers/virtio/virtio_ring.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 650f701a5480..4860787286db 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2042,6 +2042,35 @@ static struct virtqueue *vring_create_virtqueue_packed(
return NULL;
}
+static int virtqueue_resize_packed(struct virtqueue *_vq, u32 num)
+{
+ struct vring_virtqueue_packed vring = {};
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ struct virtio_device *vdev = _vq->vdev;
+ int err;
+
+ if (vring_alloc_queue_packed(&vring, vdev, num))
+ goto err_ring;
+
+ err = vring_alloc_state_extra_packed(&vring);
+ if (err)
+ goto err_state_extra;
+
+ vring_free(&vq->vq);
+
+ virtqueue_init(vq, vring.vring.num);
+ virtqueue_vring_attach_packed(vq, &vring);
+ virtqueue_vring_init_packed(vq);
+
+ return 0;
+
+err_state_extra:
+ vring_free_packed(&vring, vdev);
+err_ring:
+ virtqueue_reinit_packed(vq);
So desc_state and desc_extra has been freed vring_free_packed() when
vring_alloc_state_extra_packed() fails. We might get use-after-free here?
vring_free_packed() frees the temporary structure vring. It does not affect
desc_state and desc_extra of vq. So it is safe.
You are right.
Actually, I think for resize we need
1) detach old
2) allocate new
3) if 2) succeed, attach new otherwise attach old
The implementation is now:
1. allocate new
2. free old (detach old)
3. attach new
error:
1. free temporary
2. reinit old
Do you think this is ok? We need to add a new variable to save the old vring in
the process you mentioned, there is not much difference in other.
Yes, I think the code is fine. But I'd suggest to rename "vring" to
"vring_packed", this simplify the reviewers.
Other than this, you can add:
Acked-by: Jason Wang <jasowang@xxxxxxxxxx>
Thanks.
This seems more clearer than the current logic?
Thanks
+ return -ENOMEM;
+}
+
/*
* Generic functions and exported symbols.
