ubusd: replace ubusd_msg_unshare() with ubus_msg_new() to prevent invalid free-ing
authorAlexandru Ardelean <ardeleanalex@gmail.com>
Fri, 27 Jun 2014 16:11:43 +0000 (19:11 +0300)
committerFelix Fietkau <nbd@openwrt.org>
Thu, 3 Jul 2014 10:45:49 +0000 (12:45 +0200)
The realloc is problematic mostly with large packets, as the pointer changes
so what eventually gets free'd is invalid.
Note that ub ptr param in the  call will be passed on to a ubus_msg_free(),
right after ubus_msg_ref() finishes.

This bug stayed hidden the same way as the bug in libubus writev_retry().
Since the write/sendmsg function can send about ~200k the ubus_msg_enqueue()
call does not get triggered.

ubusd.c

diff --git a/ubusd.c b/ubusd.c
index bcc8603a60be678733ed5f4623d48eae4746a98e..89031053a2386700d1c9bf6f4eb36e8aa1d6b60f 100644 (file)
--- a/ubusd.c
+++ b/ubusd.c
 
 #include "ubusd.h"
 
-static struct ubus_msg_buf *ubus_msg_unshare(struct ubus_msg_buf *ub)
-{
-       ub = realloc(ub, sizeof(*ub) + ub->len);
-       if (!ub)
-               return NULL;
-
-       ub->refcount = 1;
-       memcpy(ub + 1, ub->data, ub->len);
-       ub->data = (void *) (ub + 1);
-       return ub;
-}
-
 static struct ubus_msg_buf *ubus_msg_ref(struct ubus_msg_buf *ub)
 {
        if (ub->refcount == ~0)
-               return ubus_msg_unshare(ub);
+               return ubus_msg_new(ub->data, ub->len, false);
 
        ub->refcount++;
        return ub;