Re: [libvirt] [PATCH] rpc : fix a access for null pointer
On 07/15/2017 05:00 PM, Peng Hao wrote:
virNetSocketRemoveIOCallback get sock's ObjectLock and will call
virNetSocketEventFree. virNetSocketEventFree may be free sock
object and virNetSocketRemoveIOCallback will access a null pointer
in release sock's ObjectLock.
Signed-off-by: Liu Yun <liu.yunh(a)zte.com.cn>
Signed-off-by: Peng Hao <peng.hao2(a)zte.com.cn>
---
src/rpc/virnetsocket.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
I don't think this can work.
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index d228c8a..8b550e8 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -2140,14 +2140,12 @@ static void virNetSocketEventFree(void *opaque)
virFreeCallback ff;
void *eopaque;
- virObjectLock(sock);
ff = sock->ff;
eopaque = sock->opaque;
sock->func = NULL;
sock->ff = NULL;
sock->opaque = NULL;
- virObjectUnlock(sock);
I think we need the lock here. This function is called from the event
loop thread. So even if virNetSocketUpdateIOCallback() locks the @socket
this code can see it unlocked. Or locked. But the crucial part is it's
modifying the object and thus should have lock held.
-
+
if (ff)
ff(eopaque);
@@ -2207,6 +2205,7 @@ void virNetSocketUpdateIOCallback(virNetSocketPtr sock,
void virNetSocketRemoveIOCallback(virNetSocketPtr sock)
{
+ virObjectRef(sock);
virObjectLock(sock);
I think this is what actually fixes your problem. However, I also think
it introduces uneven ratio of ref:unref calls.
if (sock->watch < 0) {
@@ -2220,6 +2219,7 @@ void virNetSocketRemoveIOCallback(virNetSocketPtr sock)
sock->watch = -1;
virObjectUnlock(sock);
+ virObjectRef(sock);
It definitely does so because you ref twice. Anyway, do you perhaps have
a backtrace to share?
Michal