On Wed, Sep 29, 2021 at 02:46:52PM +0300, Dan Carpenter wrote:
On Wed, Sep 29, 2021 at 02:37:42PM +0300, Dan Carpenter wrote:89 /* The last byte is the status and we checked if the last iov has 90 * enough room for it. 91 */ 92 to_push = vringh_kiov_length(&vq->in_iov) - 1; Are you positive that vringh_kiov_length() cannot be zero? I looked at the range_check() and there is no check for "if (*len == 0)". 93 94 to_pull = vringh_kiov_length(&vq->out_iov); 95 96 bytes = vringh_iov_pull_iotlb(&vq->vring, &vq->out_iov, &hdr, 97 sizeof(hdr)); 98 if (bytes != sizeof(hdr)) { 99 dev_err(&vdpasim->vdpa.dev, "request out header too short\n"); 100 return false; 101 } 102 103 to_pull -= bytes; The same "bytes" is used for both to_pull and to_push. In this assignment it would only be used for the default case which prints an error message.Sorry, no. This part is wrong. "bytes" is not used for "to_push" either here or below. But I still am not sure "*len == 0" or how we
At line 84 we check that the last `in_iov` has at least one byte, so vringh_kiov_length(&vq->in_iov) cannot be zero.
It will return the sum of all lengths, so at least 1. Maybe better to add a comment.
know that "to_push >= VIRTIO_BLK_ID_BYTES".
vringh_iov_push_iotlb() will push at least the bytes available in `in_iov`, if these are less, it will copy less bytes of VIRTIO_BLK_ID_BYTES.
Maybe here it would be better to add a check because the driver isn't following the specification.
And I'd avoid the subtraction highlighted by Smatch static checker. Thanks for reporting. I'll send patches to fix these issues. Stefano _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
