svn commit: r1402897 - in /apr/apr/trunk: CHANGES buckets/apr_brigade.c test/testbuckets.c

[sf...@apache.org]

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 <christophe jaillet wanadoo fr>]
 

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