On Donnerstag, 19. Dezember 2019 23:36:07 CET Stefano Stabellini wrote:
> On Thu, 19 Dec 2019, Christian Schoenebeck wrote:
> > On Donnerstag, 19. Dezember 2019 01:42:51 CET Stefano Stabellini wrote:
> > > From: Stefano Stabellini <stefano.stabell...@xilinx.com>
> > >
> > > init_in_iov_from_pdu might not be able to allocate the full buffer size
> > > requested, which comes from the client and could be larger than the
> > > transport has available at the time of the request. Specifically, this
> > > can happen with read operations, with the client requesting a read up to
> > > the max allowed, which might be more than the transport has available at
> > > the time.
> >
> > I haven't looked thoroughly at this yet, but that's about addressing a
> > temporary, not a permanent transport buffer size limitation, right?
>
> Yes, that is correct.
One more thing ...
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 775e8ff766..68873c3f5f 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -145,19 +145,15 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu,
> size_t offset, }
>
> static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> - unsigned int *pniov, size_t size)
> + unsigned int *pniov, size_t *size)
> {
> V9fsState *s = pdu->s;
> V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> VirtQueueElement *elem = v->elems[pdu->idx];
> size_t buf_size = iov_size(elem->in_sg, elem->in_num);
>
> - if (buf_size < size) {
> - VirtIODevice *vdev = VIRTIO_DEVICE(v);
> -
> - virtio_error(vdev,
> - "VirtFS reply type %d needs %zu bytes, buffer has
> %zu", - pdu->id + 1, size, buf_size);
> + if (buf_size < *size) {
> + *size = buf_size;
> }
>
> *piov = elem->in_sg;
Here could be a problem: what happens if the currently available transport
buffer size is extremely small, i.e. less than P9_IOHDRSZ? I am not sure that
would be handled safely everywhere. So maybe it would make sense to make
transport buffer size < P9_IOHDRSZ an error case here?
Best regards,
Christian Schoenebeck
