Peter Xu <[email protected]> writes: > 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. >
Remember to copy Oracle folks. >> >> > } >> > >> > static int cpr_exec_find_state(void) >>
