From 98ca307749606d4208c3f868fbc08de3a9f33994 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Thu, 16 May 2019 09:50:43 +0300 Subject: [PATCH 1/3] test: cover adjusting of last_with_datap in evbuffer_prepend() Before the fix: $ regress evbuffer/empty_reference_prepend.. evbuffer/empty_reference_prepend: [forking] FAIL ../test/regress_buffer.c:104: assert(chain == buf->first) FAIL ../test/regress_buffer.c:2291: Buffer format invalid [empty_reference_prepend FAILED] evbuffer/empty_reference_prepend_buffer: [forking] OK 1/2 TESTS FAILED. (0 skipped) --- test/regress_buffer.c | 54 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/regress_buffer.c b/test/regress_buffer.c index 479488a0..f57d8c44 100644 --- a/test/regress_buffer.c +++ b/test/regress_buffer.c @@ -2240,6 +2240,58 @@ end: } +static void +test_evbuffer_empty_reference_prepend(void *ptr) +{ + struct evbuffer *buf = NULL; + + buf = evbuffer_new(); + tt_assert(buf); + + /** empty chain could leave invalid last_with_datap */ + evbuffer_add_reference(buf, "", 0, NULL, NULL); + evbuffer_validate(buf); + evbuffer_prepend(buf, "foo", 3); + + evbuffer_validate(buf); + tt_assert(!strncmp((char *)evbuffer_pullup(buf, -1), "foo", 3)); + evbuffer_validate(buf); + +end: + if (buf) + evbuffer_free(buf); +} +static void +test_evbuffer_empty_reference_prepend_buffer(void *ptr) +{ + struct evbuffer *buf1 = NULL, *buf2 = NULL; + + buf1 = evbuffer_new(); + tt_assert(buf1); + buf2 = evbuffer_new(); + tt_assert(buf2); + + /** empty chain could leave invalid last_with_datap */ + evbuffer_add_reference(buf1, "", 0, NULL, NULL); + evbuffer_validate(buf1); + evbuffer_add(buf2, "foo", 3); + evbuffer_validate(buf2); + evbuffer_prepend_buffer(buf2, buf1); + evbuffer_validate(buf2); + + tt_assert(!strncmp((char *)evbuffer_pullup(buf2, -1), "foo", 3)); + evbuffer_validate(buf2); + + tt_assert(!strncmp((char *)evbuffer_pullup(buf1, -1), "", 0)); + evbuffer_validate(buf2); + +end: + if (buf1) + evbuffer_free(buf1); + if (buf2) + evbuffer_free(buf2); +} + static void test_evbuffer_peek_first_gt(void *info) { @@ -2654,6 +2706,8 @@ struct testcase_t evbuffer_testcases[] = { { "multicast", test_evbuffer_multicast, 0, NULL, NULL }, { "multicast_drain", test_evbuffer_multicast_drain, 0, NULL, NULL }, { "prepend", test_evbuffer_prepend, TT_FORK, NULL, NULL }, + { "empty_reference_prepend", test_evbuffer_empty_reference_prepend, TT_FORK, NULL, NULL }, + { "empty_reference_prepend_buffer", test_evbuffer_empty_reference_prepend_buffer, TT_FORK, NULL, NULL }, { "peek", test_evbuffer_peek, 0, NULL, NULL }, { "peek_first_gt", test_evbuffer_peek_first_gt, 0, NULL, NULL }, { "freeze_start", test_evbuffer_freeze, 0, &nil_setup, (void*)"start" }, From 244cacaf8ccfe8b8978df20791998bbc92225dc8 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Thu, 16 May 2019 09:42:41 +0300 Subject: [PATCH 2/3] test: regression for evbuffer_expand_fast_() with invalid last_with_datap Before the fix: $ regress --no-fork evbuffer/reserve_invalid_last_with_datap evbuffer/empty_chain_expand: [err] ../buffer.c:2138: Assertion chain == buf->first failed in evbuffer_expand_fast_ Aborted (core dumped) This is the a shorter version of test from the #806 (with some comments). --- test/regress_buffer.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/regress_buffer.c b/test/regress_buffer.c index f57d8c44..02d557b6 100644 --- a/test/regress_buffer.c +++ b/test/regress_buffer.c @@ -63,6 +63,8 @@ #include "regress.h" +#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0])) + /* Validates that an evbuffer is good. Returns false if it isn't, true if it * is*/ static int @@ -757,6 +759,41 @@ test_evbuffer_reserve_with_empty(void *ptr) evbuffer_free(buf); } +/* regression for evbuffer_expand_fast_() with invalid last_with_datap that has + * been left after evbuffer_prepend() with empty chain in it */ +static void +test_evbuffer_reserve_invalid_last_with_datap(void *ptr) +{ + struct evbuffer *buf = NULL; + struct evbuffer_iovec vec[2]; + const int nvec = ARRAY_SIZE(vec); + int i, avec; + + buf = evbuffer_new(); + tt_assert(buf); + + /* prepend with an empty chain */ + evbuffer_add_reference(buf, "", 0, NULL, NULL); + evbuffer_prepend(buf, "foo", 3); + /* after invalid last_with_datap will create new chain */ + evbuffer_add(buf, "", 0); + /* we need to create at least 2 "used" (in evbuffer_expand_fast_()) chains */ + tt_int_op(avec = evbuffer_reserve_space(buf, 1<<12, vec, nvec), >=, 1); + for (i = 0; i < avec; ++i) + vec[i].iov_len = 0; + tt_int_op(evbuffer_commit_space(buf, vec, avec), ==, 0); + + /* and an actual problem, that triggers an assert(chain == buf->first) in + * evbuffer_expand_fast_() */ + tt_int_op(evbuffer_reserve_space(buf, 1<<13, vec, nvec), >=, 1); + + evbuffer_validate(buf); + +end: + if (buf) + evbuffer_free(buf); +} + static void test_evbuffer_expand(void *ptr) { @@ -2689,6 +2726,7 @@ struct testcase_t evbuffer_testcases[] = { { "reserve_many2", test_evbuffer_reserve_many, 0, &nil_setup, (void*)"add" }, { "reserve_many3", test_evbuffer_reserve_many, 0, &nil_setup, (void*)"fill" }, { "reserve_with_empty", test_evbuffer_reserve_with_empty, 0, NULL, NULL }, + { "reserve_invalid_last_with_datap", test_evbuffer_reserve_invalid_last_with_datap, TT_FORK, NULL, NULL }, { "expand", test_evbuffer_expand, 0, NULL, NULL }, { "expand_overflow", test_evbuffer_expand_overflow, 0, NULL, NULL }, { "add1", test_evbuffer_add1, 0, NULL, NULL }, From 401bd1c09e9753ce997539c177cb1e48025a7343 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 11 May 2019 00:17:03 +0300 Subject: [PATCH 3/3] evbuffer: fix last_with_datap after prepend with empty chain last_with_datap should be adjusted only if it buf->first *was* not empty, otherwise last_with_datap should point to the prepended chain. --- buffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buffer.c b/buffer.c index 11b1bdae..6ad8fd24 100644 --- a/buffer.c +++ b/buffer.c @@ -1905,7 +1905,7 @@ evbuffer_prepend(struct evbuffer *buf, const void *data, size_t datlen) if ((tmp = evbuffer_chain_new(datlen)) == NULL) goto done; buf->first = tmp; - if (buf->last_with_datap == &buf->first) + if (buf->last_with_datap == &buf->first && chain->off) buf->last_with_datap = &tmp->next; tmp->next = chain;