On Wed, Sep 29, 2021 at 02:07:12PM +0200, Stefano Garzarella wrote:
> 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.
Nothing to fix, really. I've looked at what you've explained and it's
all true so the code is fine as-is. Thanks so much.
regards,
dan carpenter
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
