>-----Original Message----- >From: Geoff Thorpe [mailto:[EMAIL PROTECTED] [SNIP] > >Just one :-) I hadn't been particularly clear about something >so wires may >have got crossed, there is a second patch lurking around and >it's purpose >is overlapped with the one you posted. The patch you sent reduces the >memcpy() overhead to the minimum required whereas previously it was >pegged at the maximum possible. The cost for that is the addition of >another member variable in the index structure. However the use of >"maximal" memcpy over "minimum" memcpy was not the bug, just an >inelegance of the code. The real bug was that no check was being made >that the size of the desired memcpy was less than the size of the >(sub-)cache, no matter whether it was maximal or minimal! :-) >I think the >bug would have been triggered by maximal and minimal >scenarios, provided >you used small enough cache sizes (less than 256kb) and waited long >enough.
Well.. This SEGV bug has to happen the very first time if and ONLY if the session data is bigger than the cache size - it may never happen otherwise. Moreover the problem does not show up when we're writing the session data into the cache for the first time - it's the first lookup that causes the problem. This is because when we write to the cache, we use the 'encoded_len' as the size - which is smaller than the cache size and is the correct method. The patch you posted fixes the case where encoded_len is bigger than the cache size. So, we definitely require the patch. However, for retrieval of the session data, we assume a standard size (SSL_SESSION_MAX_DER) - this is wrong (in our case). Your patch certainly fixes the problem - but is NOT a optimal fix for the problem. We should retrieve only the same number of bytes that we've written - for both correctness and performance. As regards the patch attached, I'd propose to move the checking in the initialization routine (buf_size should be atleast SSL_SESSION_MAX_DER) rather than checking everytime (for bettter performance) -Madhu
