[PATCH] virdomainjob: virDomainObjInitJob: Store a copy of virDomainObjPrivateJobCallbacks

[Ján Tomko]

On a Monday in 2022, Peter Krempa wrote:
>'virDomainObjPrivateJobCallbacks' is passed into the job object by
>copying a pointer from the 'virDomainXMLOption' struct passed in from
>the caller. Unfortunately the 'virdomainjob' module can't control the
>lifetime of the virDomainXMLOption, which in some cases is freed before
>the domain job data.
>
>To avoid dereferencing freed memory create a copy of the struct holding
>the private job callbacks.
>
>Fixes: 84e9fd068ccad6e19e037cd6680df437617e2de5
>Signed-off-by: Peter Krempa <pkrempa at redhat.com>
>---
>This is a naive attempt to fix a crash reported on the upstream mailing
>list
>
>https://listman.redhat.com/archives/libvir-list/2022-September/234310.html
>
>Note that I didn't analyze the code in that series in detail, I just
>debugged the visible misbehaviour.
>
>CI pipeline now passes even on OpenSUSE:
>
>https://gitlab.com/pipo.sk/libvirt/-/pipelines/643828256
>
> src/conf/virdomainjob.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/src/conf/virdomainjob.c b/src/conf/virdomainjob.c
>index 7915faa125..16fb0c2177 100644
>--- a/src/conf/virdomainjob.c
>+++ b/src/conf/virdomainjob.c
>@@ -122,13 +122,25 @@ virDomainJobStatusToType(virDomainJobStatus status)
>     return VIR_DOMAIN_JOB_NONE;
> }
>
>+
>+static virDomainObjPrivateJobCallbacks *
>+virDomainObjPrivateJobCallbacksCopy(virDomainObjPrivateJobCallbacks *cb)
>+{
>+    virDomainObjPrivateJobCallbacks *ret = g_new0(virDomainObjPrivateJobCallbacks, 1);
>+
>+    memcpy(ret, cb, sizeof(virDomainObjPrivateJobCallbacks));
>+
>+    return ret;
>+}
>+
>+
> int
> virDomainObjInitJob(virDomainJobObj *job,
>                     virDomainObjPrivateJobCallbacks *cb,
>                     virDomainJobDataPrivateDataCallbacks *jobDataPrivateCb)
> {
>     memset(job, 0, sizeof(*job));
>-    job->cb = cb;
>+    job->cb = virDomainObjPrivateJobCallbacksCopy(cb);
>     job->jobDataPrivateCb = jobDataPrivateCb;
>

For the only callers in QEMU driver, jobDataPrivateCb has the same
lifetime as cb, so they both need the same treatment.

Jano

>     if (virCondInit(&job->cond) < 0)