On Thu, Jul 10, 2025 at 04:46:34PM +0100, Peter Maydell wrote:
> Hi; Coverity complains about a potential filedescriptor leak in
> net/vhost-vdpa.c:net_init_vhost_vdpa(). This is CID 1490785.
>
> Specifically, in this function we do:
> queue_pairs = vhost_vdpa_get_max_queue_pairs(vdpa_device_fd, features,
> &has_cvq, errp);
> if (queue_pairs < 0) {
> [exit with failure]
> }
> ...
> ncs = g_malloc0(sizeof(*ncs) * queue_pairs);
> for (i = 0; i < queue_pairs; i++) {
> ...
> ncs[i] = net_vhost_vdpa_init(..., vdpa_device_fd, ...)
> ...
> }
> if (has_cvq) {
> ...
> nc = net_host_vdpa_init(..., vdpa_device_fd, ...)
> ...
> }
>
> So if queue_pairs is zero we will malloc(0) which seems dubious;
> and if queue_pairs is zero and has_cvq is false then the init
> function will exit success without ever calling net_vhost_vdpa_init()
> and it will leak the vdpa_device_fd.
>
> My guess is that queue_pairs == 0 should be an error, or possibly
> that (queue_pairs == 0 && !has_cvq) should be an error.
>
> Could somebody who knows more about this code tell me which, and
> perhaps produce a patch to make it handle that case?
Historically queue_pairs == 0 was always same as 1, IIRC.
> Q: should this file be listed in the "vhost" subcategory of MAINTAINERS?
> At the moment it only gets caught by "Network device backends".
>
> thanks
> -- PMM
This so.
