Bugs item #1911069, was opened at 2008-03-10 16:31
Message generated for change (Comment added) made by imercury
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=1911069&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: crash
Status: Open
Resolution: None
Priority: 7
Private: No
Submitted By: Andrey (imercury)
Assigned to: Daniel Stenberg (bagder)
Summary: Race Condition in Curl_resolv(hostip.c) (dns cache)

Initial Comment:
Hello.

I've noticed a bug in Curl_resolv which can lead to crash of curl(or bad behaviour)(I've checked versions 7.16.1 and 7.18.0, but I think older versions are also buggy). Look at hostip.c:423 (curl 7.18.0). Consider we're sharing dns cache between 2 threads(A and B).
A: locks dns cache
A: gets dns from cache
A: unlocks dns cache
B: locks dns cache
B: gets dns from cache
B: unlock dns cache
A: checks remove_entry_if_stale and it returns true
.
So dns entry is removed from cache and thread A knows it, so it sets dns = NULL; but thread B doesn't know it and it has dns variable pointed to nobody knows what...

My solution for that, diff hostip_old.c hostip.c (v7.18.0) is attached. Not sure it's ok, but it worked on 7.16.1 (at least I didn't get curl crashed).

Regards, Andrew..

----------------------------------------------------------------------

>Comment By: Andrey (imercury)
Date: 2008-03-11 20:17

Message:
Logged In: YES
user_id=2032292
Originator: YES

Seems ok to me

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2008-03-11 16:05

Message:
Logged In: YES
user_id=1110
Originator: NO

Unfortunately, your approach is not good enough either since it also uses
'dns' outside the locks without having it "bumped" so there's still a small
chance that the other thread prunes that entry exactly at that point.

I did a change that makes sure to do inuse++ within the lock so that the
race can be avoided. It makes test 506 fail, but no other and 506 is
expected to fail since this patch changes the number of locks used.

Can you please try this version and tell us how it works?
File Added: Curl_resolv-race.patch

----------------------------------------------------------------------

Comment By: Andrey (imercury)
Date: 2008-03-11 14:58

Message:
Logged In: YES
user_id=2032292
Originator: YES

Not sure it's ok though. It just works in my case..
File Added: mydiff

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2008-03-11 01:33

Message:
Logged In: YES
user_id=1110
Originator: NO

Thanks a lot, but can you please make the diff with -u and submit such a
version?

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=1911069&group_id=976
Received on 2008-03-11