You are viewing a plain text version of this content. The canonical link for it is here.
Posted to test-dev@httpd.apache.org by Cliff Woolley <cl...@yahoo.com> on 2001/08/24 22:39:32 UTC
Re: cvs commit: httpd-2.0/modules/http http_protocol.c
If someone's feeling ambitious, they could add more tests for this sort of
thing... with bucket brigades being as they are, many of the cases that
could fail do so with the "odd" bucket types like pipe and socket.
It just occurred to me that by the time we arrive at this particular
block, the byterange filter should have normalized all such buckets
because it calls apr_brigade_length(), which reads in all buckets of
indeterminate length. So this block is probably never reached, but it's
worthwhile to have it right anyway.
Anyhow, it's entirely possible that there are other places in the code
that ARE reached and that break when they get pipe and socket buckets...
--Cliff
On 24 Aug 2001 jwoolley@apache.org wrote:
> jwoolley 01/08/24 13:27:40
>
> Modified: modules/http http_protocol.c
> Log:
> Fix a double-free condition when byterange requests are made on brigades
> containing any bucket that cannot be copied natively (ie, pipe or socket
> buckets).
>
> Before, we were reading that bucket to morph it to a heap bucket and then
> taking the str that heap bucket points to and placing it in a second,
> completely separate heap bucket. That means we'd have two apr_bucket/
> apr_bucket_heap pairs each with a refcount of 1 (rather than two apr_buckets
> and a single apr_bucket_heap with a refcount of 2). str would then be
> doubly-freed when the second of those two buckets was destroyed.
>
> Revision Changes Path
> 1.355 +6 -1 httpd-2.0/modules/http/http_protocol.c
>
> Index: http_protocol.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
> retrieving revision 1.354
> retrieving revision 1.355
> diff -u -d -u -r1.354 -r1.355
> --- http_protocol.c 2001/08/16 03:58:16 1.354
> +++ http_protocol.c 2001/08/24 20:27:40 1.355
> @@ -2468,8 +2468,13 @@
> apr_size_t len;
>
> if (apr_bucket_copy(ec, &foo) != APR_SUCCESS) {
> + /* we assume here that if copy failed we can morph
> + * the bucket into a copyable one by reading it... normally
> + * copy won't return anything but APR_SUCCESS or APR_ENOTIMPL
> + */
> + /* XXX: check for failure? */
> apr_bucket_read(ec, &str, &len, APR_BLOCK_READ);
> - foo = apr_bucket_heap_create(str, len, 0, NULL);
> + apr_bucket_copy(ec, &foo);
> }
> APR_BRIGADE_INSERT_TAIL(bsend, foo);
> ec = APR_BUCKET_NEXT(ec);
>
>
>
>
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: cvs commit: httpd-2.0/modules/http http_protocol.c
Posted by Greg Stein <gs...@lyra.org>.
On Fri, Aug 24, 2001 at 04:39:32PM -0400, Cliff Woolley wrote:
>...
> It just occurred to me that by the time we arrive at this particular
> block, the byterange filter should have normalized all such buckets
> because it calls apr_brigade_length(), which reads in all buckets of
> indeterminate length. So this block is probably never reached, but it's
> worthwhile to have it right anyway.
It should be right. It would be very easy for a bucket to be based on a pipe
or a socket, thus being uncopyable, but it knows the length of data to be
read from that pipe/socket.
Imagine if you have a particular protocol running over some socket to a
backend server. You know the next chunk of data is 1000 bytes. You insert
your custom socket bucket, saying "<this> socket, 1000 bytes". It can be
read, but it can't be copied.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: httpd-2.0/modules/http http_protocol.c
Posted by Greg Stein <gs...@lyra.org>.
On Fri, Aug 24, 2001 at 04:39:32PM -0400, Cliff Woolley wrote:
>...
> It just occurred to me that by the time we arrive at this particular
> block, the byterange filter should have normalized all such buckets
> because it calls apr_brigade_length(), which reads in all buckets of
> indeterminate length. So this block is probably never reached, but it's
> worthwhile to have it right anyway.
It should be right. It would be very easy for a bucket to be based on a pipe
or a socket, thus being uncopyable, but it knows the length of data to be
read from that pipe/socket.
Imagine if you have a particular protocol running over some socket to a
backend server. You know the next chunk of data is 1000 bytes. You insert
your custom socket bucket, saying "<this> socket, 1000 bytes". It can be
read, but it can't be copied.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/