[libvirt] [PATCH 1/4] remote_daemon_dispatch: Check for limit properly in remoteDispatchConnectGetAllDomainStats

[Jonathon Jongsma]

Looks correct to me. Good catch.

Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>


On Tue, 2019-08-27 at 14:05 +0200, Michal Privoznik wrote:
> The return structure is a bit complicated and that's why it is
> very easy to check for RPC limits incorrectly. The structure is
> an array of remote_domain_stats_record structures with the limit
> of REMOTE_DOMAIN_LIST_MAX. The latter structure then poses a
> different limit on typed params:
> REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX (which is what we are
> checking for mistakenly).
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/remote/remote_daemon_dispatch.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/src/remote/remote_daemon_dispatch.c
> b/src/remote/remote_daemon_dispatch.c
> index fb8b95f315..1a002957ef 100644
> --- a/src/remote/remote_daemon_dispatch.c
> +++ b/src/remote/remote_daemon_dispatch.c
> @@ -6996,15 +6996,15 @@
> remoteDispatchConnectGetAllDomainStats(virNetServerPtr server
> ATTRIBUTE_UNUSED,
>              goto cleanup;
>      }
>  
> -    if (nrecords > REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Number of domain stats records is %d, "
> -                         "which exceeds max limit: %d"),
> -                       nrecords,
> REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX);
> -        goto cleanup;
> -    }
> -
>      if (nrecords) {
> +        if (nrecords > REMOTE_DOMAIN_LIST_MAX) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Number of domain stats records is %d,
> "
> +                             "which exceeds max limit: %d"),
> +                           nrecords, REMOTE_DOMAIN_LIST_MAX);
> +            goto cleanup;
> +        }
> +
>          if (VIR_ALLOC_N(ret->retStats.retStats_val, nrecords) < 0)
>              goto cleanup;
>