From 7a1e907fa7cadf386c9ce449e1f382df39811cec Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Fri, 16 Jun 2017 11:49:22 +1000 Subject: [PATCH] Rename json_set_object to json_steal_object to make it clear what the function does and nullify the original pointer preventing use after free. --- src/connector.c | 8 ++++---- src/generator.c | 12 ++++++------ src/libckpool.h | 9 +++++---- src/stratifier.c | 32 ++++++++++++++++---------------- 4 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/connector.c b/src/connector.c index b7973d5a..d5257558 100644 --- a/src/connector.c +++ b/src/connector.c @@ -1378,7 +1378,7 @@ char *connector_stats(void *data, const int runtime) ck_runlock(&cdata->lock); JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", generated); - json_set_object(val, "clients", subval); + json_steal_object(val, "clients", subval); ck_rlock(&cdata->lock); DL_COUNT(cdata->dead_clients, client, objects); @@ -1387,7 +1387,7 @@ char *connector_stats(void *data, const int runtime) memsize = objects * sizeof(client_instance_t); JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", generated); - json_set_object(val, "dead", subval); + json_steal_object(val, "dead", subval); objects = 0; memsize = 0; @@ -1398,12 +1398,12 @@ char *connector_stats(void *data, const int runtime) memsize += sizeof(sender_send_t) + send->len + 1; } JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", cdata->sends_generated); - json_set_object(val, "sends", subval); + json_steal_object(val, "sends", subval); JSON_CPACK(subval, "{si,si,si}", "count", cdata->sends_queued, "memory", cdata->sends_size, "generated", cdata->sends_delayed); mutex_unlock(&cdata->sender_lock); - json_set_object(val, "delays", subval); + json_steal_object(val, "delays", subval); buf = json_dumps(val, JSON_NO_UTF8 | JSON_PRESERVE_ORDER); json_decref(val); diff --git a/src/generator.c b/src/generator.c index 23c46803..d94e88a4 100644 --- a/src/generator.c +++ b/src/generator.c @@ -2834,12 +2834,12 @@ static void send_stats(gdata_t *gdata, const int sockd) memsize = SAFE_HASH_OVERHEAD(gdata->proxies) + sizeof(proxy_instance_t) * objects; generated = gdata->proxies_generated; JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", generated); - json_set_object(val, "proxies", subval); + json_steal_object(val, "proxies", subval); DL_COUNT(gdata->dead_proxies, proxy, objects); memsize = sizeof(proxy_instance_t) * objects; JSON_CPACK(subval, "{si,si}", "count", objects, "memory", memsize); - json_set_object(val, "dead_proxies", subval); + json_steal_object(val, "dead_proxies", subval); total_objects = memsize = 0; for (proxy = gdata->proxies; proxy; proxy=proxy->hh.next) { @@ -2852,7 +2852,7 @@ static void send_stats(gdata_t *gdata, const int sockd) mutex_unlock(&gdata->lock); JSON_CPACK(subval, "{si,si,si}", "count", total_objects, "memory", memsize, "generated", generated); - json_set_object(val, "subproxies", subval); + json_steal_object(val, "subproxies", subval); mutex_lock(&gdata->notify_lock); objects = HASH_COUNT(gdata->notify_instances); @@ -2861,7 +2861,7 @@ static void send_stats(gdata_t *gdata, const int sockd) mutex_unlock(&gdata->notify_lock); JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", generated); - json_set_object(val, "notifies", subval); + json_steal_object(val, "notifies", subval); mutex_lock(&gdata->share_lock); objects = HASH_COUNT(gdata->shares); @@ -2870,7 +2870,7 @@ static void send_stats(gdata_t *gdata, const int sockd) mutex_unlock(&gdata->share_lock); JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", generated); - json_set_object(val, "shares", subval); + json_steal_object(val, "shares", subval); mutex_lock(&gdata->psend_lock); DL_COUNT(gdata->psends, msg, objects); @@ -2879,7 +2879,7 @@ static void send_stats(gdata_t *gdata, const int sockd) memsize = sizeof(stratum_msg_t) * objects; JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", generated); - json_set_object(val, "psends", subval); + json_steal_object(val, "psends", subval); send_api_response(val, sockd); } diff --git a/src/libckpool.h b/src/libckpool.h index 478794e9..306f1c6c 100644 --- a/src/libckpool.h +++ b/src/libckpool.h @@ -409,14 +409,15 @@ static inline void _json_set_bool(json_t *val, const char *key, bool boolean, } #define json_set_bool(val, key, boolean) _json_set_bool(val, key, boolean, __FILE__, __func__, __LINE__) - -static inline void _json_set_object(json_t *val, const char *key, json_t *object, +/* Steals an object and NULLs original reference */ +static inline void _json_steal_object(json_t *val, const char *key, json_t **object, const char *file, const char *func, const int line) { - if (unlikely(json_object_set_new_nocheck(val, key, object))) + if (unlikely(json_object_set_new_nocheck(val, key, *object))) LOGERR("Failed to set json object from %s %s:%d", file, func, line); + *object = NULL; } -#define json_set_object(val, key, object) _json_set_object(val, key, object, __FILE__, __func__, __LINE__) +#define json_steal_object(val, key, object) _json_steal_object(val, key, &(object), __FILE__, __func__, __LINE__) void rename_proc(const char *name); void create_pthread(pthread_t *thread, void *(*start_routine)(void *), void *arg); diff --git a/src/stratifier.c b/src/stratifier.c index 0c539d5a..2727e9b1 100644 --- a/src/stratifier.c +++ b/src/stratifier.c @@ -3827,32 +3827,32 @@ char *stratifier_stats(ckpool_t *ckp, void *data) memsize = SAFE_HASH_OVERHEAD(sdata->workbases) + sizeof(workbase_t) * objects; generated = sdata->workbases_generated; JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", generated); - json_set_object(val, "workbases", subval); + json_steal_object(val, "workbases", subval); objects = HASH_COUNT(sdata->remote_workbases); memsize = SAFE_HASH_OVERHEAD(sdata->remote_workbases) + sizeof(workbase_t) * objects; ck_runlock(&sdata->workbase_lock); JSON_CPACK(subval, "{si,si}", "count", objects, "memory", memsize); - json_set_object(val, "remote_workbases", subval); + json_steal_object(val, "remote_workbases", subval); ck_rlock(&sdata->instance_lock); objects = HASH_COUNT(sdata->user_instances); memsize = SAFE_HASH_OVERHEAD(sdata->user_instances) + sizeof(stratum_instance_t) * objects; JSON_CPACK(subval, "{si,si}", "count", objects, "memory", memsize); - json_set_object(val, "users", subval); + json_steal_object(val, "users", subval); objects = HASH_COUNT(sdata->stratum_instances); memsize = SAFE_HASH_OVERHEAD(sdata->stratum_instances); generated = sdata->stratum_generated; JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", generated); - json_set_object(val, "clients", subval); + json_steal_object(val, "clients", subval); objects = sdata->stats.disconnected; generated = sdata->disconnected_generated; memsize = SAFE_HASH_OVERHEAD(sdata->disconnected_sessions); memsize += sizeof(session_t) * sdata->stats.disconnected; JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", generated); - json_set_object(val, "disconnected", subval); + json_steal_object(val, "disconnected", subval); ck_runlock(&sdata->instance_lock); mutex_lock(&sdata->share_lock); @@ -3862,27 +3862,27 @@ char *stratifier_stats(ckpool_t *ckp, void *data) mutex_unlock(&sdata->share_lock); JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", generated); - json_set_object(val, "shares", subval); + json_steal_object(val, "shares", subval); ck_rlock(&sdata->txn_lock); objects = HASH_COUNT(sdata->txns); memsize = SAFE_HASH_OVERHEAD(sdata->txns) + sizeof(txntable_t) * objects; generated = sdata->txns_generated; JSON_CPACK(subval, "{si,si,si}", "count", objects, "memory", memsize, "generated", generated); - json_set_object(val, "transactions", subval); + json_steal_object(val, "transactions", subval); ck_runlock(&sdata->txn_lock); ckmsgq_stats(sdata->ssends, sizeof(smsg_t), &subval); - json_set_object(val, "ssends", subval); + json_steal_object(val, "ssends", subval); /* Don't know exactly how big the string is so just count the pointer for now */ ckmsgq_stats(sdata->srecvs, sizeof(char *), &subval); - json_set_object(val, "srecvs", subval); + json_steal_object(val, "srecvs", subval); if (!CKP_STANDALONE(ckp)) { ckmsgq_stats(sdata->ckdbq, sizeof(char *), &subval); - json_set_object(val, "ckdbq", subval); + json_steal_object(val, "ckdbq", subval); } ckmsgq_stats(sdata->stxnq, sizeof(json_params_t), &subval); - json_set_object(val, "stxnq", subval); + json_steal_object(val, "stxnq", subval); buf = json_dumps(val, JSON_NO_UTF8 | JSON_PRESERVE_ORDER); json_decref(val); @@ -7539,12 +7539,12 @@ static void ssend_process(ckpool_t *ckp, smsg_t *msg) free(msg); } +/* json_decref on NULL is safe */ static void discard_json_params(json_params_t *jp) { json_decref(jp->method); json_decref(jp->params); - if (jp->id_val) - json_decref(jp->id_val); + json_decref(jp->id_val); free(jp); } @@ -7618,9 +7618,9 @@ static void upstream_auth(ckpool_t *ckp, stratum_instance_t *client, json_params ts_realtime(&now); sprintf(cdfield, "%lu,%lu", now.tv_sec, now.tv_nsec); - json_set_object(val, "params", jp->params); - json_set_object(val, "id", jp->id_val); - json_set_object(val, "method", jp->method); + json_steal_object(val, "params", jp->params); + json_steal_object(val, "id", jp->id_val); + json_steal_object(val, "method", jp->method); json_set_string(val, "method", stratum_msgs[SM_AUTH]); json_set_string(val, "useragent", client->useragent ? : "");