httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mathihalli, Madhusudan" <>
Subject RE: [PATCH] Fix SEGV in ssl_scache_shmcb.c
Date Tue, 11 May 2004 00:50:01 GMT

>-----Original Message-----
>From: Geoff Thorpe [] 
>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 

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
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

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)


View raw message