'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@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;

     if (virCondInit(&job->cond) < 0)
@@ -229,6 +241,8 @@ virDomainObjClearJob(virDomainJobObj *job)

     if (job->cb && job->cb->freeJobPrivate)
         g_clear_pointer(&job->privateData, job->cb->freeJobPrivate);
+
+    g_clear_pointer(&job->cb, g_free);
 }

 void
-- 
2.37.1

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@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)