Re: [PATCH v3 1/6] libvhost-user: Add vu_rem_mem_reg input validation

[David Hildenbrand <david@redhat.com>]

On 17.01.22 05:12, Raphael Norwitz wrote:
> Today if multiple FDs are sent from the VMM to the backend in a
> VHOST_USER_REM_MEM_REG message, one FD will be unmapped and the remaining
> FDs will be leaked. Therefore if multiple FDs are sent we report an
> error and fail the operation, closing all FDs in the message.
> 
> Likewise in case the VMM sends a message with a size less than that of a
> memory region descriptor, we add a check to gracefully report an error
> and fail the operation rather than crashing.
> 
> Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> ---
>  subprojects/libvhost-user/libvhost-user.c | 15 +++++++++++++++
>  subprojects/libvhost-user/libvhost-user.h |  2 ++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
> index 787f4d2d4f..b09b1c269e 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -801,6 +801,21 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
>      VuDevRegion shadow_regions[VHOST_USER_MAX_RAM_SLOTS] = {};
>      VhostUserMemoryRegion m = vmsg->payload.memreg.region, *msg_region = &m;
>  
> +    if (vmsg->fd_num != 1) {
> +        vmsg_close_fds(vmsg);
> +        vu_panic(dev, "VHOST_USER_REM_MEM_REG received %d fds - only 1 fd "
> +                      "should be sent for this message type", vmsg->fd_num);
> +        return false;
> +    }
> +
> +    if (vmsg->size < VHOST_USER_MEM_REG_SIZE) {
> +        close(vmsg->fds[0]);

I wonder if using vmsg_close_fds(vmsg); makes the code easier to read.

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb