Descriptor count in virtio_vring_info and fw_rsc_vdev_vring are differing data types, leading to possible truncation on assignment

[tammyleino]

num is a uint32_t in fw_rsc_vdev_vring, but this value gets cast to uint16_t by rproc_virtio_init_vring when assigning num_descs to vring_info->info.num_descs.

It is improbable that someone would configure uint32_t number of descriptors, but it would be more correct for these two data members to match in type.

[arnopo]

Right!
as the vring_alloc_info structure contains a padding field following update could be done smoothly:

struct vring_alloc_info {
	void *vaddr;
	uint32_t align;
-	uint16_t num_descs;
-	uint16_t pad;
+	uint32_t num_descs;
};

could you send a PR to propose it?

[tammyleino]

@arnopo Yes, will do. I have a couple PRs I'm going to push out together maybe next week - week after at the latest.

[tammyleino]

@arnopo Changing num_descs to uint32_t will also require vq_nentries in the virtqueue struct to change to uint32_t (otherwise we're just pushing the truncation further up), which causes additional fallout throughout the code with data type changes. Is it even feasible that an application would want more buffers than 65,535? If no, I suggest we not make this change. We cannot change num in fw_rsc_vdev_vring to be uint16_t or the resource tables won't be compatible going forward, but can we document that num can only be 16-bits in length?

[arnopo]

Having more than 65,535 does not seem like a realistic use case to me.
In short term we could also add a metal_assert in rproc_virtio_init_vring with an inline associated comment.

[tammyleino]

#407
#403