Re: [PATCH 2/3] migration/cpr: Fix coverity report in cpr_exec_persist_state()

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> Per reported and analyzed by Peter:
>
> https://lore.kernel.org/r/CAFEAcA_mUQ2NeoguR5efrhw7XYGofnriWEA=+Dg+Ocvyam1wAw@mail.gmail.com
>
> mfd leak is a false positive, try to use a coverity annotation (which I
> didn't find manual myself, but still give it a shot).
>
> Fix the other one by dumping an error message if setenv() failed.
>
> Resolves: Coverity CID 1641391
> Resolves: Coverity CID 1641392
> Fixes: efc6587313 ("migration: cpr-exec save and load")
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/cpr-exec.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
> index d57714bc5d..3cf44634a9 100644
> --- a/migration/cpr-exec.c
> +++ b/migration/cpr-exec.c
> @@ -43,13 +43,16 @@ static QEMUFile *qemu_file_new_fd_output(int fd, const char *name)
>  void cpr_exec_persist_state(QEMUFile *f)
>  {
>      QIOChannelFile *fioc = QIO_CHANNEL_FILE(qemu_file_get_ioc(f));
> +    /* coverity[leaked_storage] - mfd intentionally kept open across exec() */
>      int mfd = dup(fioc->fd);
>      char val[16];
>  
>      /* Remember mfd in environment for post-exec load */
>      qemu_clear_cloexec(mfd);
>      snprintf(val, sizeof(val), "%d", mfd);
> -    g_setenv(CPR_EXEC_STATE_NAME, val, 1);
> +    if (!g_setenv(CPR_EXEC_STATE_NAME, val, 1)) {
> +        error_report("Setting env %s = %s failed", CPR_EXEC_STATE_NAME, val);
> +    }

Best to abort no? We don't want the rest of the code reading whatever
may be at that env variable and running with it.

>  }
>  
>  static int cpr_exec_find_state(void)