Author: sf Date: Sat Oct 27 21:56:58 2012 New Revision: 1402897 URL: http://svn.apache.org/viewvc?rev=1402897&view=rev Log: Fix potential data corruption in apr_brigade_write() and friends if the last bucket of the brigade is a heap bucket that has been split, and there are still references to the next part of the original bucket in use. Modified: apr/apr/trunk/CHANGES apr/apr/trunk/buckets/apr_brigade.c apr/apr/trunk/test/testbuckets.c Modified: apr/apr/trunk/CHANGES URL: http://svn.apache.org/viewvc/apr/apr/trunk/CHANGES?rev=1402897&r1=1402896&r2=1402897&view=diff ============================================================================== --- apr/apr/trunk/CHANGES [utf-8] (original) +++ apr/apr/trunk/CHANGES [utf-8] Sat Oct 27 21:56:58 2012 @@ -1,6 +1,11 @@ -*- coding: utf-8 -*- Changes for APR 2.0.0 + *) Fix potential data corruption in apr_brigade_write() and friends if + the last bucket of the brigade is a heap bucket that has been split, + and there are still references to the next part of the original bucket + in use. [Stefan Fritsch] + *) Remove duplicated logic in apr_brigade_puts(). PR 53740. [Christophe Jaillet ] Modified: apr/apr/trunk/buckets/apr_brigade.c URL: http://svn.apache.org/viewvc/apr/apr/trunk/buckets/apr_brigade.c?rev=1402897&r1=1402896&r2=1402897&view=diff ============================================================================== --- apr/apr/trunk/buckets/apr_brigade.c (original) +++ apr/apr/trunk/buckets/apr_brigade.c Sat Oct 27 21:56:58 2012 @@ -448,7 +448,12 @@ APR_DECLARE(apr_status_t) apr_brigade_wr apr_size_t remaining = APR_BUCKET_BUFF_SIZE; char *buf = NULL; - if (!APR_BRIGADE_EMPTY(b) && APR_BUCKET_IS_HEAP(e)) { + /* + * If the last bucket is a heap bucket and its buffer is not shared with + * another bucket, we may write into that bucket. + */ + if (!APR_BRIGADE_EMPTY(b) && APR_BUCKET_IS_HEAP(e) + && ((apr_bucket_heap *)(e->data))->refcount.refcount == 1) { apr_bucket_heap *h = e->data; /* HEAP bucket start offsets are always in-memory, safe to cast */ @@ -538,10 +543,11 @@ APR_DECLARE(apr_status_t) apr_brigade_wr i = 0; /* If there is a heap bucket at the end of the brigade - * already, copy into the existing bucket. + * already, and its refcount is 1, copy into the existing bucket. */ e = APR_BRIGADE_LAST(b); - if (!APR_BRIGADE_EMPTY(b) && APR_BUCKET_IS_HEAP(e)) { + if (!APR_BRIGADE_EMPTY(b) && APR_BUCKET_IS_HEAP(e) + && ((apr_bucket_heap *)(e->data))->refcount.refcount == 1) { apr_bucket_heap *h = e->data; apr_size_t remaining = h->alloc_len - (e->length + (apr_size_t)e->start); Modified: apr/apr/trunk/test/testbuckets.c URL: http://svn.apache.org/viewvc/apr/apr/trunk/test/testbuckets.c?rev=1402897&r1=1402896&r2=1402897&view=diff ============================================================================== --- apr/apr/trunk/test/testbuckets.c (original) +++ apr/apr/trunk/test/testbuckets.c Sat Oct 27 21:56:58 2012 @@ -469,6 +469,25 @@ static void test_partition(abts_case *tc apr_bucket_alloc_destroy(ba); } +static void test_write_split(abts_case *tc, void *data) +{ + apr_bucket_alloc_t *ba = apr_bucket_alloc_create(p); + apr_bucket_brigade *bb1 = apr_brigade_create(p, ba); + apr_bucket_brigade *bb2; + apr_bucket *e; + + e = apr_bucket_heap_create(hello, strlen(hello), NULL, ba); + APR_BRIGADE_INSERT_HEAD(bb1, e); + apr_bucket_split(e, strlen("hello, ")); + bb2 = apr_brigade_split(bb1, APR_BRIGADE_LAST(bb1)); + apr_brigade_write(bb1, NULL, NULL, "foo", strlen("foo")); + test_bucket_content(tc, APR_BRIGADE_FIRST(bb2), "world", 5); + + apr_brigade_destroy(bb1); + apr_brigade_destroy(bb2); + apr_bucket_alloc_destroy(ba); +} + abts_suite *testbuckets(abts_suite *suite) { suite = ADD_SUITE(suite); @@ -484,6 +503,7 @@ abts_suite *testbuckets(abts_suite *suit abts_run_test(suite, test_manyfile, NULL); abts_run_test(suite, test_truncfile, NULL); abts_run_test(suite, test_partition, NULL); + abts_run_test(suite, test_write_split, NULL); return suite; }