Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (athena.apache.org: domain of jorton@redhat.com designates
 66.187.233.31 as permitted sender)
Date: Mon, 25 Feb 2008 11:01:18 +0000
From: Joe Orton <jorton@redhat.com>
To: dev@httpd.apache.org
Subject: Re: svn commit: r630307 - in /httpd/httpd/trunk/modules/ssl:
	ssl_private.h ssl_scache.c ssl_scache_dbm.c ssl_scache_dc.c
	ssl_scache_memcache.c ssl_scache_shmcb.c
Message-ID: <20080225110118.GB7265@redhat.com>
Mail-Followup-To: dev@httpd.apache.org
References: <20080222195845.B995D1A983A@eris.apache.org>
 <47BFF81A.1060705@apache.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <47BFF81A.1060705@apache.org>
User-Agent: Mutt/1.5.17 (2007-11-01)
Organization: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor,
 Berkshire, SL4 1TE, United Kingdom.
  Registered in UK and Wales under Company Registration No. 03798903
  Directors: Michael Cunningham (USA), Brendan Lane (Ireland),
 Matt Parson (USA), Charlie Peters (USA)

On Sat, Feb 23, 2008 at 11:40:26AM +0100, Ruediger Pluem wrote:
> On 02/22/2008 08:58 PM, jorton@apache.org wrote:
>> Author: jorton
>> Date: Fri Feb 22 11:58:39 2008
>> New Revision: 630307
>>
>> URL: http://svn.apache.org/viewvc?rev=630307&view=rev
...
>>      memcpy(&expiry, dbmval.dptr, sizeof(time_t));
>> +    memcpy(dest, (char *)dbmval.dptr + sizeof(time_t), nData);
>
> Shouldn't we do
>
> *destlen = nData;
>
> here?

Fixed both of those cases in r630787.

>> ==============================================================================
>> --- httpd/httpd/trunk/modules/ssl/ssl_scache_shmcb.c (original)
>> +++ httpd/httpd/trunk/modules/ssl/ssl_scache_shmcb.c Fri Feb 22 11:58:39 2008
...
>> +        /* Only consider 'idx' if the id matches, and the "removed"
>> +         * flag isn't set; check the data length too to avoid a buffer
>> +         * overflow in case of corruption, which should be impossible,
>> +         * but it's cheap to be safe. */
>> +        if (idx->id_len == idlen && (idx->data_used - idx->id_len) < *destlen
>> +            && shmcb_cyclic_memcmp(header->subcache_data_size,
>> +                                   SHMCB_DATA(header, subcache),
>> +                                   idx->data_pos, id, idx->id_len) == 0) {
>
> Where do you check for the removed flag?

And both of those cases in r630786.

Thanks a lot for the careful review!

joe