Bug#534683: libssl0.9.8: IMPL_CHECK gives a helgrind error
Package: libssl0.9.8
Version: 0.9.8g-15+lenny1
Severity: normal
==27415== Possible data race during read of size 8 at 0x55ef9c8 by thread #4
==27415==at 0x52D1046: CRYPTO_new_ex_data (ex_data.c:570)
==27415==by 0x5318BD7: RSA_new_method (rsa_lib.c:185)
==27415==by 0x531B76C: rsa_cb (rsa_asn1.c:80)
==27415==by 0x534CB42: asn1_item_ex_combine_new (tasn_new.c:177)
==27415==by 0x53501E4: ASN1_item_ex_d2i (tasn_dec.c:399)
==27415==by 0x53502B3: ASN1_item_d2i (tasn_dec.c:134)
==27415==by 0x534863C: d2i_PublicKey (d2i_pu.c:96)
==27415==by 0x534624F: X509_PUBKEY_get (x_pubkey.c:364)
==27415==by 0x5346C07: d2i_PUBKEY (x_pubkey.c:390)
==27415==by 0x40D480: SelectorInfo::Parse(char*) (dkimverify.cpp:1312)
==27415==by 0x40E0A4: CDKIMVerify::GetSelector(std::string const, std::stri
ng const) (dkimverify.cpp:1369)
==27415==by 0x410120: CDKIMVerify::ProcessHeaders() (dkimverify.cpp:719)
==27415== This conflicts with a previous write of size 8 by thread #2
==27415==at 0x52D0F67: impl_check (ex_data.c:205)
==27415==by 0x52D1084: CRYPTO_new_ex_data (ex_data.c:570)
==27415==by 0x532684F: BIO_set (bio_lib.c:100)
==27415==by 0x53268D9: BIO_new (bio_lib.c:76)
==27415==by 0x5326E81: BIO_new_mem_buf (bss_mem.c:102)
==27415==by 0x4065C4: dk_end (domainkeys.c:1843)
==27415==by 0x406D22: dk_eom (domainkeys.c:1982)
==27415==by 0x4034CC: domainkeys_verify(int, char const*, int, unsigned char
**, char***) (dkim-test.cpp:218)
I get the above an on AMD64 system. Line 570 of ex_data.c has IMPL_CHECK which
is defined as follows:
/* Internal function that checks whether impl is set and if not, sets it to
* the default. */
static void impl_check(void)
{
CRYPTO_w_lock(CRYPTO_LOCK_EX_DATA);
if(!impl)
impl = impl_default;
CRYPTO_w_unlock(CRYPTO_LOCK_EX_DATA);
}
/* A macro wrapper for impl_check that first uses a non-locked test before
* invoking the function (which checks again inside a lock). */
#define IMPL_CHECK if(!impl) impl_check();
So if we changed the macro definition to the following then the problem would
go away:
#define IMPL_CHECK impl_check();
But that would probably decrease performance. Is there a possibility of a
pointer write not being atomic?
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#534683: [Pkg-openssl-devel] Bug#534683: libssl0.9.8: IMPL_CHECK gives a helgrind error
On Fri, Jun 26, 2009 at 08:22:06PM +1000, Russell Coker wrote:
/* Internal function that checks whether impl is set and if not, sets it to
* the default. */
static void impl_check(void)
{
CRYPTO_w_lock(CRYPTO_LOCK_EX_DATA);
if(!impl)
impl = impl_default;
CRYPTO_w_unlock(CRYPTO_LOCK_EX_DATA);
}
/* A macro wrapper for impl_check that first uses a non-locked test before
* invoking the function (which checks again inside a lock). */
#define IMPL_CHECK if(!impl) impl_check();
So if we changed the macro definition to the following then the problem would
go away:
#define IMPL_CHECK impl_check();
But that would probably decrease performance. Is there a possibility of a
pointer write not being atomic?
The answer to that question is very CPU specific. There is no
guarantee that it is atomic. And I'm not even sure that being
atomic is a good enough guarantee. You probably also need
to prevent reordering (with barriers).
Looking at the Linux kernel, they have a define/function
atomic_set() just to be able to write a integer atomicly on all
arches it supports. There is also an atomic_read().
Looking at this example, impl could be in the process of being
written in thread 1, but only half written, and then thread 2's
if (!impl) could fail and it might call impl() with a wrong
pointer. It's probably unlikely that this happens, but it
always bites you sooner or later.
Kurt
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
