[PATCH] virdomainjob: preserveJob: memdup the cb structure instead of copying it
In case of variable 'oldjob' (job structure) in qemuProcessReconnect() the init function was not called and the cb pointer was just copied from the existing job structure in virDomainObjPreserveJob(). This caused that the job and oldjob had the same pointer, which was later freed at the end of the qemuProcessReconnect() function by automatic call to virDomainObjClearJob(). This caused an invalid read in case of a daemon crash as the job structure was trying to read cb which had been already freed. This patch changes the copying to g_memdup that allocates different pointer, which can be later safely freed. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/virdomainjob.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/virdomainjob.c b/src/conf/virdomainjob.c index aca801af38..0c67e84ef1 100644 --- a/src/conf/virdomainjob.c +++ b/src/conf/virdomainjob.c @@ -210,7 +210,7 @@ virDomainObjPreserveJob(virDomainJobObj *currJob, if (currJob->cb && currJob->cb->allocJobPrivate && !(currJob->privateData = currJob->cb->allocJobPrivate())) return -1; - job->cb = currJob->cb; + job->cb = g_memdup(currJob->cb, sizeof(*currJob->cb)); virDomainObjResetJob(currJob); virDomainObjResetAsyncJob(currJob); -- 2.37.3
On 9/29/22 13:56, Kristina Hanicova wrote:
In case of variable 'oldjob' (job structure) in qemuProcessReconnect() the init function was not called and the cb pointer was just copied from the existing job structure in virDomainObjPreserveJob(). This caused that the job and oldjob had the same pointer, which was later freed at the end of the qemuProcessReconnect() function by automatic call to virDomainObjClearJob(). This caused an invalid read in case of a daemon crash as the job structure was trying to read cb which had been already freed.
This patch changes the copying to g_memdup that allocates different pointer, which can be later safely freed.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/virdomainjob.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Cleaned up the commit message a bit. Specifically, using virDomainObjInitJob() wouldn't really help here, because job->cb and currJob->cb would still share the same pointer after virDomainObjPreserveJob() is called. In fact, it would lead to a memory leak because the first thing that virDomainObjPreserveJob() does is memset() job to 0. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Michal
participants (2)
-
Kristina Hanicova -
Michal Prívozník