qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-block] [PATCH 1/3] blockjob: fix dead pointer in


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 1/3] blockjob: fix dead pointer in txn list
Date: Tue, 2 Aug 2016 14:05:52 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0

On 02.08.2016 01:39, John Snow wrote:
On 07/27/2016 06:49 AM, Vladimir Sementsov-Ogievskiy wrote:
Job may be freed in block_job_unref and in this case this would break
transaction QLIST.

Fix this by removing job from this list before unref.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
 blockjob.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/blockjob.c b/blockjob.c
index a5ba3be..e045091 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -216,6 +216,7 @@ static void block_job_completed_single(BlockJob *job)
     }
     job->cb(job->opaque, job->ret);
     if (job->txn) {
+        QLIST_REMOVE(job, txn_list);
         block_job_txn_unref(job->txn);
     }
     block_job_unref(job);


Has this caused actual problems for you?

Yes, with the same changed test 124 (my parallel thread). Backup job can finish too early (if dirty bitmap is empty) and then we use this transaction job list with dead pointer.


This function is only ever called in a transactional context if the transaction is over -- so we're not likely to use the pointers ever again anyway.

Backup job may finish even earlier than all jobs are added to the list. (same case, empty dirty bitmap for one of drives).


Still, it's good practice, and the caller uses a safe iteration of the list, so I think this should be safe.

But I don't think this SHOULD fix an actual bug. If it does, I think something else is wrong.

Tested-by: John Snow <address@hidden>
Reviewed-by: John Snow <address@hidden>


--
Best regards,
Vladimir




reply via email to

[Prev in Thread] Current Thread [Next in Thread]