On Fri, 28 Feb 2020 at 09:26, Juan Quintela <quint...@redhat.com> wrote:
>
> It will be used later.
>
> Signed-off-by: Juan Quintela <quint...@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
>
Hi; Coverity thinks there might be a buffer overrun here.
It's probably wrong, but it's not completely obvious why
it can't happen, so an assert somewhere would help...
(This is CID 1487239.)
> +MultiFDCompression migrate_multifd_compression(void)
> +{
> + MigrationState *s;
> +
> + s = migrate_get_current();
> +
> + return s->parameters.multifd_compression;
This function returns an enum of type MultiFDCompression,
whose (autogenerated from QAPI) definition is:
typedef enum MultiFDCompression {
MULTIFD_COMPRESSION_NONE,
MULTIFD_COMPRESSION_ZLIB,
#if defined(CONFIG_ZSTD)
MULTIFD_COMPRESSION_ZSTD,
#endif /* defined(CONFIG_ZSTD) */
MULTIFD_COMPRESSION__MAX,
} MultiFDCompression;
> @@ -604,6 +745,7 @@ int multifd_save_setup(Error **errp)
> multifd_send_state->pages = multifd_pages_init(page_count);
> qemu_sem_init(&multifd_send_state->channels_ready, 0);
> atomic_set(&multifd_send_state->exiting, 0);
> + multifd_send_state->ops = multifd_ops[migrate_multifd_compression()];
Here we take the result of the function and use it as an
array index into multifd_ops, whose definition is
static MultiFDMethods *multifd_ops[MULTIFD_COMPRESSION__MAX] = { ... }
Coverity doesn't see any reason why the return value from
migrate_multifd_compression() can't be MULTIFD_COMPRESSION__MAX,
so it complains that if it is then we are going to index off the
end of the array.
An assert in migrate_multifd_compression() that the value being
returned is within the expected range would probably placate it.
Alternatively, if the qapi type codegen didn't put the __MAX
value as a possible value of the enum type then Coverity
and probably also the compiler wouldn't consider it to be
a possible value of this kind of variable. But that might
have other awkward side-effects.
thanks
-- PMM
