On Tue, Oct 21, 2025 at 05:49:07PM -0300, Fabiano Rosas wrote:
> Peter Xu <[email protected]> writes:
> 
> > Per reported and analyzed by Peter:
> >
> > https://lore.kernel.org/r/CAFEAcA_mUQ2NeoguR5efrhw7XYGofnriWEA=+dg+ocvyam1...@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 <[email protected]>
> > ---
> >  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.

I didn't want to abort, because it's the same as QMP migrate failure at the
beginning.

If we want to do better, we can allow this function to return a failure
instead.

And.. when I was trying cpr-exec with no argv it already can crash QEMU.  I
think that's also a bug.

Let me prepare something better than this..  I'll likely add a new patch to
fix that too.

> 
> >  }
> >  
> >  static int cpr_exec_find_state(void)
> 

-- 
Peter Xu


Reply via email to