Public bug reported:
I am working on an application that concurrently opens HTTPS connections
through libcurl 7.42.1 on NSS 3.18. I checked the source code, and I
believe the bug persists in the latest release of NSS, 3.23 at the time
of writing.
I noticed occasional crashes during the first few HTTP connections
opened by the application. The crashes happened when libcurl attempted
to call function pointers that were supposed to have been initialized by
ssl_SetupIOMethods.
I eventually produced a multi-threaded core dump that showed that the
crashing thread had proceeded past the SSL initialization code while
another thread was still inside ssl_InitIOLayer, a function that is only
called by ssl_PushIOLayer, where the function call is protected by
double-checked locking and PR_CallOnce.
I believe that the issue is the plain Boolean variable called ssl_inited
that is used for double-checked locking. The compiler is free to reorder
reads and writes to plain Boolean variables, and, alas, it makes use of
that freedom in my binary. Here is proof:
(gdb) l
2729
2730static PRStatus
2731ssl_InitIOLayer(void)
2732{
2733ssl_layer_id = PR_GetUniqueIdentity("SSL");
2734ssl_SetupIOMethods();
2735ssl_inited = PR_TRUE;
2736return PR_SUCCESS;
2737}
2738
(gdb) p ssl_inited
$2 = 1
(gdb) frame
#5 ssl_InitIOLayer () at sslsock.c:2734
(gdb) disassemble
Dump of assembler code for function ssl_InitIOLayer:
0x00346f4287e0 <+0>: lea0xae39(%rip),%rdi# 0x346f433620
0x00346f4287e7 <+7>: sub$0x8,%rsp
0x00346f4287eb <+11>:callq 0x346f40a748
0x00346f4287f0 <+16>:mov%eax,0x21572a(%rip)#
0x346f63df20
0x00346f4287f6 <+22>:callq 0x346f40a208
0x00346f4287fb <+27>:mov%rax,%rsi
0x00346f4287fe <+30>:lea0x45b(%rip),%rax# 0x346f428c60
0x00346f428805 <+37>:lea0x215734(%rip),%rdi#
0x346f63df40
0x00346f42880c <+44>:mov$0x24,%ecx
0x00346f428811 <+49>:movl $0x1,0x215701(%rip)#
0x346f63df1c
=> 0x00346f42881b <+59>:rep movsq %ds:(%rsi),%es:(%rdi)
The assembly shows that ssl_inited is set to 1 before the call to
ssl_SetupIOMethods. This causes double-checked locking to fail and
allows other threads to proceed while ssl_InitIOLayer is still running.
#7 0x00346f42c6cb in ssl_PushIOLayer (ns=0x7fd48c0dd560,
stack=0x7fd48c0dd500, id=-2) at sslsock.c:2746
2746status = PR_CallOnce(, _InitIOLayer);
(gdb) l
2741{
2742PRFileDesc *layer = NULL;
2743PRStatusstatus;
2744
2745if (!ssl_inited) {
2746status = PR_CallOnce(, _InitIOLayer);
2747if (status != PR_SUCCESS)
2748goto loser;
2749}
The fix is to either remove ssl_inited and always call PR_CallOnce (at
the cost of some performance), or make sure the compiler does not
reorder accesses to ssl_inited. The best way to do the latter is
platform dependant. I assume the Mozilla libraries have some utility
code for this purpose (atomic Booleans and such).
** Affects: nss (Ubuntu)
Importance: Undecided
Status: New
--
You received this bug notification because you are a member of Ubuntu
Touch seeded packages, which is subscribed to nss in Ubuntu.
https://bugs.launchpad.net/bugs/1571133
Title:
Race condition in initialization code of ssl_PushIOLayer
Status in nss package in Ubuntu:
New
Bug description:
I am working on an application that concurrently opens HTTPS
connections through libcurl 7.42.1 on NSS 3.18. I checked the source
code, and I believe the bug persists in the latest release of NSS,
3.23 at the time of writing.
I noticed occasional crashes during the first few HTTP connections
opened by the application. The crashes happened when libcurl attempted
to call function pointers that were supposed to have been initialized
by ssl_SetupIOMethods.
I eventually produced a multi-threaded core dump that showed that the
crashing thread had proceeded past the SSL initialization code while
another thread was still inside ssl_InitIOLayer, a function that is
only called by ssl_PushIOLayer, where the function call is protected
by double-checked locking and PR_CallOnce.
I believe that the issue is the plain Boolean variable called
ssl_inited that is used for double-checked locking. The compiler is
free to reorder reads and writes to plain Boolean variables, and,
alas, it makes use of that freedom in my binary. Here is proof:
(gdb) l
2729
2730 static PRStatus
2731 ssl_InitIOLayer(void)
2732 {
2733 ssl_layer_id = PR_GetUniqueIdentity("SSL");
2734 ssl_SetupIOMethods();
2735 ssl_inited = PR_TRUE;
2736 return PR_SUCCESS;
2737 }
2738
(gdb) p ssl_inited
$2 = 1
(gdb) frame
#5 ssl_InitIOLayer () at
