On 04/20/2017 02:20 PM, Hailiang Zhang wrote:
On 2017/4/20 12:32, Zhang Chen wrote:
When network traffic heavy, compare_pri_rs_finalize() and
compare_sec_rs_finalize() have a chance to confilct.
Both of them call colo_compare_connection() to compare packet,
But during compare_pri_rs_finalize() comparison, have secondary
packet come and call compare_sec_rs_finalize(), that packet will be
handle twice. If packet same, the pkt will be double free.
Interesting, if I'm right, this should not happen, because, all the
comparing works
are done in colo compare thread, so there is no chance to access the
connect_list
concurrently. Besides, even both of the packets from primary and
secondary arrive
at the same time, it should only be handle once, we will handle it
with the later arrived one,
No ?
No, In my test often trigger this bug, you can use udp server and client
test it.
13517@1492648526.850246:colo_compare_main : packet same and release packet
13517@1492648526.850304:colo_compare_main : packet same and release packet
*** glibc detected ***
/home/zhangchen/qemu-colo-apr14/x86_64-softmmu/qemu-system-x86_64:
double free or corruption (out): 0x0000555556a75210 ***
======= Backtrace: =========
/lib64/libc.so.6(+0x76628)[0x7ffff53d6628]
/lib64/libc.so.6(cfree+0x6c)[0x7ffff53db5cc]
Thanks
Zhang Chen
Signed-off-by: Zhang Chen <zhangchen.f...@cn.fujitsu.com>
---
net/colo-compare.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 54e6d40..686c1b4 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -79,6 +79,8 @@ typedef struct CompareState {
* element type: Connection
*/
GQueue conn_list;
+ /* compare lock */
+ QemuMutex compare_lock;
/* hashtable to save connection */
GHashTable *connection_track_table;
/* compare thread, a thread for each NIC */
@@ -619,7 +621,9 @@ static void
compare_pri_rs_finalize(SocketReadState *pri_rs)
compare_chr_send(&s->chr_out, pri_rs->buf,
pri_rs->packet_len);
} else {
/* compare connection */
+ qemu_mutex_lock(&s->compare_lock);
g_queue_foreach(&s->conn_list, colo_compare_connection, s);
+ qemu_mutex_unlock(&s->compare_lock);
}
}
@@ -631,7 +635,9 @@ static void
compare_sec_rs_finalize(SocketReadState *sec_rs)
trace_colo_compare_main("secondary: unsupported packet in");
} else {
/* compare connection */
+ qemu_mutex_lock(&s->compare_lock);
g_queue_foreach(&s->conn_list, colo_compare_connection, s);
+ qemu_mutex_unlock(&s->compare_lock);
}
}
@@ -702,6 +708,7 @@ static void colo_compare_complete(UserCreatable
*uc, Error **errp)
net_socket_rs_init(&s->sec_rs, compare_sec_rs_finalize);
g_queue_init(&s->conn_list);
+ qemu_mutex_init(&s->compare_lock);
s->connection_track_table =
g_hash_table_new_full(connection_key_hash,
connection_key_equal,
@@ -771,6 +778,7 @@ static void colo_compare_finalize(Object *obj)
g_queue_foreach(&s->conn_list, colo_flush_packets, s);
g_queue_clear(&s->conn_list);
+ qemu_mutex_destroy(&s->compare_lock);
g_hash_table_destroy(s->connection_track_table);
g_free(s->pri_indev);
.
--
Thanks
Zhang Chen
