| Message ID | 1402019610-2985-10-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Jun 06, 2014 at 09:53:30AM +0800, Fam Zheng wrote: > @@ -353,19 +355,18 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, > exit(1); > } > > - if (req->elem->out_sg[0].iov_len < sizeof(req->out) || > - req->elem->in_sg[req->elem->in_num - 1].iov_len < sizeof(*req->in)) { > - error_report("virtio-blk header not in correct element"); > - exit(1); > - } > - > if (unlikely(iov_to_buf(iov, out_num, 0, &req->out, > sizeof(req->out)) != sizeof(req->out))) { > error_report("virtio-blk request outhdr too short"); > exit(1); > } > iov_discard_front(&iov, &out_num, sizeof(req->out)); > - req->in = (void *)req->elem->in_sg[req->elem->in_num - 1].iov_base; > + assert(in_iov[in_num - 1].iov_len >= > + sizeof(struct virtio_blk_inhdr)); Why use assert() when the rest of the function uses error_report() + exit(1)? Please keep the code consistent.
Il 06/06/2014 15:16, Stefan Hajnoczi ha scritto: >> > - req->in = (void *)req->elem->in_sg[req->elem->in_num - 1].iov_base; >> > + assert(in_iov[in_num - 1].iov_len >= >> > + sizeof(struct virtio_blk_inhdr)); > Why use assert() when the rest of the function uses error_report() + > exit(1)? Please keep the code consistent. Sorry, that's my fault. I suggested assert thinking that zero-length iovecs wouldn't be possible here. Paolo
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 2282e61..cd1a8a7 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -345,7 +345,9 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) { uint32_t type; + struct iovec *in_iov = req->elem->in_sg; struct iovec *iov = req->elem->out_sg; + unsigned in_num = req->elem->in_num; unsigned out_num = req->elem->out_num; if (req->elem->out_num < 1 || req->elem->in_num < 1) { @@ -353,19 +355,18 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, exit(1); } - if (req->elem->out_sg[0].iov_len < sizeof(req->out) || - req->elem->in_sg[req->elem->in_num - 1].iov_len < sizeof(*req->in)) { - error_report("virtio-blk header not in correct element"); - exit(1); - } - if (unlikely(iov_to_buf(iov, out_num, 0, &req->out, sizeof(req->out)) != sizeof(req->out))) { error_report("virtio-blk request outhdr too short"); exit(1); } iov_discard_front(&iov, &out_num, sizeof(req->out)); - req->in = (void *)req->elem->in_sg[req->elem->in_num - 1].iov_base; + assert(in_iov[in_num - 1].iov_len >= + sizeof(struct virtio_blk_inhdr)); + req->in = (void *)in_iov[in_num - 1].iov_base + + in_iov[in_num - 1].iov_len + - sizeof(struct virtio_blk_inhdr); + iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr)); type = ldl_p(&req->out.type);
out_sg is checked by iov_to_buf below, so it can be dropped. Add assert and iov_discard_back around in_sg, as the in_sg is handled in dataplane code. Signed-off-by: Fam Zheng <famz@redhat.com> --- hw/block/virtio-blk.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)