buffer: fix possible NULL dereference in evbuffer_setcb() on ENOMEM

[ @azat:

  - add return heredoc for evbuffer_setcb()
  - add unit test with event_set_mem_functions()
  - look through the report from abi-compliance-checker/abi-dumper
]

Closes: #855
(cherry picked from commit bdcade47224f154052c927aed3c363a18b37112e)

diff --git a/buffer.c b/buffer.c
index 5e3d91b56250cc1affef4f9ecee45d282d9dca74..a51b6c5f66ebc94c44ba3f3b795dcab7fe85f7d5 100644 (file)
--- a/buffer.c
+++ b/buffer.c
@@ -3304,7 +3304,7 @@ evbuffer_add_file(struct evbuffer *buf, int fd, ev_off_t offset, ev_off_t length
        return r;
 }
 
-void
+int
 evbuffer_setcb(struct evbuffer *buffer, evbuffer_cb cb, void *cbarg)
 {
        EVBUFFER_LOCK(buffer);
@@ -3315,10 +3315,15 @@ evbuffer_setcb(struct evbuffer *buffer, evbuffer_cb cb, void *cbarg)
        if (cb) {
                struct evbuffer_cb_entry *ent =
                    evbuffer_add_cb(buffer, NULL, cbarg);
+               if (!ent) {
+                       EVBUFFER_UNLOCK(buffer);
+                       return -1;
+               }
                ent->cb.cb_obsolete = cb;
                ent->flags |= EVBUFFER_CB_OBSOLETE;
        }
        EVBUFFER_UNLOCK(buffer);
+       return 0;
 }
 
 struct evbuffer_cb_entry *

diff --git a/include/event2/buffer_compat.h b/include/event2/buffer_compat.h
index 24f828c2103302017308310f2ef2b4cea5e330b3..0ce10254487d4dc2ed670aeda1bf5889a4bb89d2 100644 (file)
--- a/include/event2/buffer_compat.h
+++ b/include/event2/buffer_compat.h
@@ -90,9 +90,10 @@ typedef void (*evbuffer_cb)(struct evbuffer *buffer, size_t old_len, size_t new_
   @param cb the callback function to invoke when the evbuffer is modified,
         or NULL to remove all callbacks.
   @param cbarg an argument to be provided to the callback function
+  @return 0 if successful, or -1 on error
  */
 EVENT2_EXPORT_SYMBOL
-void evbuffer_setcb(struct evbuffer *buffer, evbuffer_cb cb, void *cbarg);
+int evbuffer_setcb(struct evbuffer *buffer, evbuffer_cb cb, void *cbarg);
 
 
 /**

diff --git a/test/regress_buffer.c b/test/regress_buffer.c
index 02d557b66c53937f369ed00dd0102fd0d18634f0..8ac4b6e038b2b140a457cc1a8994afdba38c500d 100644 (file)
--- a/test/regress_buffer.c
+++ b/test/regress_buffer.c
@@ -1956,12 +1956,12 @@ test_evbuffer_callbacks(void *ptr)
        tt_assert(cb1 != NULL);
        cb2 = evbuffer_add_cb(buf, log_change_callback, buf_out2);
        tt_assert(cb2 != NULL);
-       evbuffer_setcb(buf, self_draining_callback, NULL);
+       tt_int_op(evbuffer_setcb(buf, self_draining_callback, NULL), ==, 0);
        evbuffer_add_printf(buf, "This should get drained right away.");
        tt_uint_op(evbuffer_get_length(buf), ==, 0);
        tt_uint_op(evbuffer_get_length(buf_out1), ==, 0);
        tt_uint_op(evbuffer_get_length(buf_out2), ==, 0);
-       evbuffer_setcb(buf, NULL, NULL);
+       tt_int_op(evbuffer_setcb(buf, NULL, NULL), ==, 0);
        evbuffer_add_printf(buf, "This will not.");
        tt_str_op((const char *) evbuffer_pullup(buf, -1), ==, "This will not.");
        evbuffer_validate(buf);
@@ -1987,6 +1987,14 @@ test_evbuffer_callbacks(void *ptr)
                  "0->15; 15->11; 11->0; ");
 #endif
 
+       /* the next call to readline should fail */
+#ifndef EVENT__DISABLE_MM_REPLACEMENT
+       event_set_mem_functions(failing_malloc, realloc, free);
+       tt_int_op(evbuffer_setcb(buf, self_draining_callback, NULL), ==, -1);
+       evbuffer_validate(buf);
+       event_set_mem_functions(malloc, realloc, free);
+#endif
+
  end:
        if (buf)
                evbuffer_free(buf);