1114741 - nsRandomGenerator doesn't guard against NSS shutdown

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

nsRandomGenerator doesn't guard against NSS shutdown in GenerateRandomBytes. I believe it caused this crash:

11:56:35  WARNING -  PROCESS-CRASH | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application crashed [@ KERNELBASE.dll + 0x89ae4]
11:56:35     INFO -  Crash dump filename: c:\users\cltbld~1.t-w\appdata\local\temp\tmpdwfnv8.mozrunner\minidumps\21aec483-9151-4e6f-af3e-ed800e2add39.dmp
11:56:35     INFO -  Operating system: Windows NT
11:56:35     INFO -                    6.2.9200
11:56:35     INFO -  CPU: x86
11:56:35     INFO -       GenuineIntel family 6 model 30 stepping 5
11:56:35     INFO -       8 CPUs
11:56:35     INFO -  Crash reason:  EXCEPTION_BREAKPOINT
11:56:35     INFO -  Crash address: 0x77949ae4
11:56:35     INFO -  Thread 0 (crashed)
11:56:35     INFO -   0  KERNELBASE.dll + 0x89ae4
11:56:35     INFO -      eip = 0x77949ae4   esp = 0x00fac838   ebp = 0x00fac848   ebx = 0x6f7d203c
11:56:35     INFO -      esi = 0x6f831ec6   edi = 0x6f7d2048   eax = 0x00000000   ecx = 0x6f83ff12
11:56:35     INFO -      edx = 0x00faac40   efl = 0x00000206
11:56:35     INFO -      Found by: given as instruction pointer in context
11:56:35     INFO -   1  nss3.dll!PK11_GetInternalSlot [pk11slot.c:d88622dd6622 : 1799 + 0x1c]
11:56:35     INFO -      eip = 0x6f5c5319   esp = 0x00fac850   ebp = 0x00fac860
11:56:35     INFO -      Found by: previous frame's frame pointer
11:56:35     INFO -   2  xul.dll!nsRandomGenerator::GenerateRandomBytes(unsigned int,unsigned char * *) [nsRandomGenerator.cpp:d88622dd6622 : 28 + 0x4]
11:56:35     INFO -      eip = 0x732b78bd   esp = 0x00fac868   ebp = 0x00fac878
11:56:35     INFO -      Found by: call frame info
11:56:35     INFO -   3  xul.dll!mozilla::net::WebSocketChannel::SetupRequest() [WebSocketChannel.cpp:d88622dd6622 : 2324 + 0x19]
11:56:35     INFO -      eip = 0x7194906d   esp = 0x00fac880   ebp = 0x00fac91c
11:56:35     INFO -      Found by: call frame info
11:56:35     INFO -   4  xul.dll!mozilla::net::WebSocketChannel::AsyncOpen(nsIURI *,nsACString_internal const &,nsIWebSocketListener *,nsISupports *) [WebSocketChannel.cpp:d88622dd6622 : 2926 + 0x6]
11:56:35     INFO -      eip = 0x71949a0a   esp = 0x00fac924   ebp = 0x00fac960
11:56:35     INFO -      Found by: call frame info
11:56:35     INFO -   5  xul.dll!NS_InvokeByIndex [xptcinvoke.cpp:d88622dd6622 : 70 + 0x2]
11:56:35     INFO -      eip = 0x7178f1cf   esp = 0x00fac968   ebp = 0x00fac98c
11:56:35     INFO -      Found by: call frame info
11:56:35     INFO -   6  xul.dll!CallMethodHelper::Invoke() [XPCWrappedNative.cpp:d88622dd6622 : 2396 + 0xe]
11:56:35     INFO -      eip = 0x71d204eb   esp = 0x00fac994   ebp = 0x00fac9b8
11:56:35     INFO -      Found by: call frame info

http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-win32-debug/1419271168/mozilla-beta_win8-debug_test-mochitest-browser-chrome-2-bm111-tests1-windows-build31.txt.gz

Comment 1

[Legacy TBPL/Treeherder Robot]

submit_timestamp: 2014-12-22T14:01:46
log: https://treeherder.mozilla.org/ui/logviewer.html#?repo=mozilla-beta&job_id=190996
repository: mozilla-beta
who: rvandermeulen[at]mozilla[dot]com
machine: t-w864-ix-056
buildname: WINNT 6.2 mozilla-beta debug test mochitest-browser-chrome-2
revision: d88622dd6622

0[2b0f140]: Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
192 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application terminated with exit code 1
PROCESS-CRASH | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application crashed [@ KERNELBASE.dll + 0x89ae4]
TEST-UNEXPECTED-FAIL | leakcheck | default process: missing output line for total leaks!
Return code: 1

Comment 2

[Legacy TBPL/Treeherder Robot]

submit_timestamp: 2014-12-26T05:52:18
log: https://treeherder.mozilla.org/ui/logviewer.html#?repo=mozilla-beta&job_id=196386
repository: mozilla-beta
who: rvandermeulen[at]mozilla[dot]com
machine: t-w732-ix-002
buildname: Windows 7 32-bit mozilla-beta debug test mochitest-browser-chrome-2
revision: 90ef4a7c4986

0[170f140]: Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
192 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application terminated with exit code 1
PROCESS-CRASH | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application crashed [@ KERNELBASE.dll + 0x3381b]
TEST-UNEXPECTED-FAIL | leakcheck | default process: missing output line for total leaks!
Return code: 1

Comment 3

[Legacy TBPL/Treeherder Robot]

submit_timestamp: 2015-01-05T05:51:16
log: https://treeherder.mozilla.org/ui/logviewer.html#?repo=mozilla-beta&job_id=201233
repository: mozilla-beta
who: rvandermeulen[at]mozilla[dot]com
machine: t-w732-ix-013
buildname: Windows 7 32-bit mozilla-beta debug test mochitest-browser-chrome-2
revision: 936d2a5ac91b

0[d11140]: Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
192 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application terminated with exit code 1
PROCESS-CRASH | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application crashed [@ KERNELBASE.dll + 0x34906]
TEST-UNEXPECTED-FAIL | leakcheck | default process: missing output line for total leaks!
Return code: 1

Comment 4

[Legacy TBPL/Treeherder Robot]

submit_timestamp: 2015-01-05T05:51:16
log: https://treeherder.mozilla.org/ui/logviewer.html#?repo=mozilla-beta&job_id=201800
repository: mozilla-beta
who: rvandermeulen[at]mozilla[dot]com
machine: t-w732-ix-066
buildname: Windows 7 32-bit mozilla-beta debug test mochitest-browser-chrome-2
revision: 936d2a5ac91b

0[1e11140]: Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
192 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application terminated with exit code 1
PROCESS-CRASH | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application crashed [@ KERNELBASE.dll + 0x3381b]
TEST-UNEXPECTED-FAIL | leakcheck | default process: missing output line for total leaks!
Return code: 1

Comment 5

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Attachment: patch

JC, if you're not already familiar with our NSS-shutdown-prevention mechanisms, this would be a good time to learn. Most of the background information is here: https://hg.mozilla.org/mozilla-central/annotate/912036eeb024/security/manager/ssl/src/nsNSSShutDown.h#l166
See also bugs with titles that mention NSS shutdown. Also, feel free to ask for clarification.

Comment 6

[J.C. Jones [:jcj] (he/him)]

Comment on attachment 8544281 [details] [diff] [review]
patch

Review of attachment 8544281 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. We all know that I comment too much, but I'd toss one in to nsRandomGenerator.cpp:23 that the purpose of nsRandomGenerator being a shutdown object is the shutdown prevention lock. But again, whatever convention says is best!

Comment 7

[Legacy TBPL/Treeherder Robot]

submit_timestamp: 2015-01-07T07:36:26
log: https://treeherder.mozilla.org/ui/logviewer.html#?repo=mozilla-release&job_id=29960
repository: mozilla-release
who: rvandermeulen[at]mozilla[dot]com
machine: t-w732-ix-131
buildname: Windows 7 32-bit mozilla-release debug test mochitest-browser-chrome-2
revision: 7c41e83e678c

0[1f11140]: Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-rel-w32-d-000000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-rel-w32-d-000000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
192 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application terminated with exit code 1
PROCESS-CRASH | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application crashed [@ KERNELBASE.dll + 0x3381b]
TEST-UNEXPECTED-FAIL | leakcheck | default process: missing output line for total leaks!
Return code: 1

Comment 8

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Thanks, JC. I don't think a comment is necessary, so I didn't add one. The only reason anything would be a shutdown object is to prevent/handle NSS shutdown via a shutdown prevention lock.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=32b2894d0928
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5889fb4ad1e

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f5889fb4ad1e

Comment 10

[Legacy TBPL/Treeherder Robot]

submit_timestamp: 2015-01-07T15:52:15
log: https://treeherder.mozilla.org/ui/logviewer.html#?repo=mozilla-beta&job_id=206512
repository: mozilla-beta
who: rvandermeulen[at]mozilla[dot]com
machine: t-w732-ix-069
buildname: Windows 7 32-bit mozilla-beta debug test mochitest-browser-chrome-2
revision: 3129d08edd20

0[1e11140]: Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
192 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application terminated with exit code 1
PROCESS-CRASH | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application crashed [@ KERNELBASE.dll + 0x3381b]
TEST-UNEXPECTED-FAIL | leakcheck | default process: missing output line for total leaks!
Return code: 1

Comment 11

[Legacy TBPL/Treeherder Robot]

submit_timestamp: 2015-01-07T15:52:16
log: https://treeherder.mozilla.org/ui/logviewer.html#?repo=mozilla-beta&job_id=206605
repository: mozilla-beta
who: rvandermeulen[at]mozilla[dot]com
machine: t-w732-ix-069
buildname: Windows 7 32-bit mozilla-beta debug test mochitest-browser-chrome-2
revision: 3129d08edd20

0[2011140]: Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
Assertion failure: mod != NULL, at c:/builds/moz2_slave/m-beta-w32-d-00000000000000000/build/security/nss/lib/pk11wrap/pk11slot.c:1799
192 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application terminated with exit code 1
PROCESS-CRASH | chrome://mochitests/content/browser/netwerk/test/browser/browser_child_resource.js | application crashed [@ KERNELBASE.dll + 0x3381b]
TEST-UNEXPECTED-FAIL | leakcheck | default process: missing output line for total leaks!
Return code: 1