Would the attached patch be agreeable to both of you? It contains a
fix (okay, it's a hack) to ensure OpenSSL doesn't loop infinitely on
crashing Heap32Next, so that should at least cover the (theoretical?)
issue of arbitrary/unknown fault origin from within Heap32Next.
Then there's the fun part about the argument of not using it in a
multithread app when it's crashing on WinCE: well, this particular bit
of code is part of the entropy harvester, which has some different
demands, compared to 'regular application code' (compare the Debian
screwup from last year), so my position is this:
as long as the crash is controlled (and it is, thanks to the code
posted by Tanguy) and it's cause is generally known (it's documented
in MSDN for one of the Microsoft platforms), it's good enough as a
partial source of entropy: after all, we do NOT depend on the data
[structures] returned, all we are looking for here, is _entropy_. (And
philosophically, is a corrupt data structure bad from an entropy
perspective? Nope. Not at all.)
Thus, this whole issue reduces to the question: do we can uncontrolled
core dumps from using this?
Answer: no, we don't.
Can we get infinite loops, assuming worst case scenario of total heap
API failure?
Maybe.
So we fix that by limiting the number of crash failures we tolerate here.
(And, yes, I know, _theoretically_, a corrupt heap [API] can deliver
seemingly 'good' structure indefinitely. Heck, same fix: limit the
number of _total_ rounds instead of the number of failures and we're
good to go. (move the ex_cnt_limit decrement to the while condition
instead, then.) Meanwhile, this is a definite software improvement for
CE, so I'd really like to have it in there.)
(Attached diff against latest 0.9.9 CVS HEAD)
--
Met vriendelijke groeten / Best regards,
Ger Hobbelt
--------------------------------------------------
web: http://www.hobbelt.com/
http://www.hebbut.net/
mail: g...@hobbelt.com
mobile: +31-6-11 120 978
--------------------------------------------------
--- /home/ger/prj/1original/openssl/openssl/./crypto/rand/rand_win.c 2005-10-13 21:06:43.000000000 +0200
+++ ./crypto/rand/rand_win.c 2009-03-11 12:25:50.000000000 +0100
@@ -494,12 +496,28 @@
* each entry. Consider each field a source of 1 byte
* of entropy.
*/
+ ZeroMemory(&hlist, sizeof(HEAPLIST32));
hlist.dwSize = sizeof(HEAPLIST32);
if (good) stoptime = GetTickCount() + MAXDELAY;
if (heaplist_first(handle, &hlist))
+ {
+ /*
+ following discussion on dev ML, exception on WinCE (or other Win
+ platform) is theoretically of unknown origin; prevent infinite
+ loop here when this theoretical case occurs; otherwise cope with
+ the expected (MSDN documented) exception-throwing behaviour of
+ Heap32Next() on WinCE.
+
+ based on patch in original message by Tanguy Fautré (2009/03/02)
+ Subject: RAND_poll() and CreateToolhelp32Snapshot() stability
+ */
+ int ex_cnt_limit = 42;
do
{
RAND_add(&hlist, hlist.dwSize, 3);
+ __try
+ {
+ ZeroMemory(&hentry, sizeof(HEAPENTRY32));
hentry.dwSize = sizeof(HEAPENTRY32);
if (heap_first(&hentry,
hlist.th32ProcessID,
@@ -512,8 +530,16 @@
while (heap_next(&hentry)
&& --entrycnt > 0);
}
- } while (heaplist_next(handle,
- &hlist) && GetTickCount() < stoptime);
+ }
+ __except (EXCEPTION_EXECUTE_HANDLER)
+ {
+ /* ignore access violations when walking the heap list */
+ ex_cnt_limit--;
+ }
+ } while (heaplist_next(handle, &hlist)
+ && GetTickCount() < stoptime
+ && ex_cnt_limit > 0);
+ }
/* process walking */
/* PROCESSENTRY32 contains 9 fields that will change
