From a9d6fc571d08de45ac3b9cfb78db9007f7b8ed48 Mon Sep 17 00:00:00 2001 From: Joshua Colp Date: Mon, 14 Dec 2015 14:04:15 -0400 Subject: [PATCH] json: Audit ast_json_* usage for thread safety. The JSON library Asterisk uses, jansson, is not thread safe for us in a few ways. To help with this wrappers for JSON object reference count increasing and decreasing were added which use a global lock to ensure they don't clobber over each other. This does not extend to reference count manipulation within the jansson library itself. This means you can't safely use the object borrowing specifier (O) in ast_json_pack and you can't share JSON instances between objects. This change removes uses of the O specifier and replaces them with the o specifier and an explicit ast_json_ref. Some cases of instance sharing have also been removed. ASTERISK-25601 #close Change-Id: I06550d8b0cc1bfeb56cab580a4e608ae4f1ec7d1 --- main/aoc.c | 20 ++++++++++---------- main/loader.c | 4 ++-- main/rtp_engine.c | 14 +++++++------- main/stasis.c | 4 ++-- main/stasis_channels.c | 24 +++++++++++++++++------- res/res_fax.c | 4 ++-- res/res_stasis.c | 6 +++--- res/res_stasis_playback.c | 4 ++-- res/res_stasis_recording.c | 4 ++-- res/stasis/app.c | 4 ++-- 10 files changed, 49 insertions(+), 39 deletions(-) diff --git a/main/aoc.c b/main/aoc.c index 5bce066515..29c5e87b61 100644 --- a/main/aoc.c +++ b/main/aoc.c @@ -1667,11 +1667,11 @@ static struct ast_json *charge_to_json(const struct ast_aoc_decoded *decoded) } return ast_json_pack( - "{s:s, s:s, s:s, s:O}", + "{s:s, s:s, s:s, s:o}", "Type", aoc_charge_type_str(decoded->charge_type), "BillingID", aoc_billingid_str(decoded->billing_id), "TotalType", aoc_type_of_totaling_str(decoded->total_type), - obj_type, obj); + obj_type, ast_json_ref(obj)); } static struct ast_json *association_to_json(const struct ast_aoc_decoded *decoded) @@ -1738,10 +1738,10 @@ static struct ast_json *s_to_json(const struct ast_aoc_decoded *decoded) "Scale", decoded->aoc_s_entries[i].rate.duration.granularity_time_scale); } - type = ast_json_pack("{s:O, s:s, s:O, s:O}", "Currency", currency, "ChargingType", + type = ast_json_pack("{s:o, s:s, s:o, s:o}", "Currency", ast_json_ref(currency), "ChargingType", decoded->aoc_s_entries[i].rate.duration.charging_type ? - "StepFunction" : "ContinuousCharging", "Time", time, - "Granularity", granularity ? granularity : ast_json_null()); + "StepFunction" : "ContinuousCharging", "Time", ast_json_ref(time), + "Granularity", granularity ? ast_json_ref(granularity) : ast_json_ref(ast_json_null())); break; } @@ -1751,7 +1751,7 @@ static struct ast_json *s_to_json(const struct ast_aoc_decoded *decoded) decoded->aoc_s_entries[i].rate.flat.amount, decoded->aoc_s_entries[i].rate.flat.multiplier); - type = ast_json_pack("{s:O}", "Currency", currency); + type = ast_json_pack("{s:o}", "Currency", ast_json_ref(currency)); break; case AST_AOC_RATE_TYPE_VOLUME: currency = currency_to_json( @@ -1760,9 +1760,9 @@ static struct ast_json *s_to_json(const struct ast_aoc_decoded *decoded) decoded->aoc_s_entries[i].rate.volume.multiplier); type = ast_json_pack( - "{s:s, s:O}", "Unit", aoc_volume_unit_str( + "{s:s, s:o}", "Unit", aoc_volume_unit_str( decoded->aoc_s_entries[i].rate.volume.volume_unit), - "Currency", currency); + "Currency", ast_json_ref(currency)); break; case AST_AOC_RATE_TYPE_SPECIAL_CODE: type = ast_json_pack("{s:i}", "SpecialCode", @@ -1772,8 +1772,8 @@ static struct ast_json *s_to_json(const struct ast_aoc_decoded *decoded) break; } - rate = ast_json_pack("{s:s, s:O}", "Chargeable", charge_item, - aoc_rate_type_str(decoded->aoc_s_entries[i].rate_type), type); + rate = ast_json_pack("{s:s, s:o}", "Chargeable", charge_item, + aoc_rate_type_str(decoded->aoc_s_entries[i].rate_type), ast_json_ref(type)); if (ast_json_array_append(rates, rate)) { break; } diff --git a/main/loader.c b/main/loader.c index 940b53edb9..2b3bd1fba5 100644 --- a/main/loader.c +++ b/main/loader.c @@ -852,10 +852,10 @@ static void publish_reload_message(const char *name, enum ast_module_reload_resu event_object = ast_json_pack("{s: s, s: s}", "Module", S_OR(name, "All"), "Status", res_buffer); - json_object = ast_json_pack("{s: s, s: i, s: O}", + json_object = ast_json_pack("{s: s, s: i, s: o}", "type", "Reload", "class_type", EVENT_FLAG_SYSTEM, - "event", event_object); + "event", ast_json_ref(event_object)); if (!json_object) { return; diff --git a/main/rtp_engine.c b/main/rtp_engine.c index beda8cd745..d659b98097 100644 --- a/main/rtp_engine.c +++ b/main/rtp_engine.c @@ -2008,12 +2008,12 @@ static struct ast_json *rtcp_report_to_json(struct stasis_message *msg, } } - json_rtcp_report = ast_json_pack("{s: i, s: i, s: i, s: O, s: O}", + json_rtcp_report = ast_json_pack("{s: i, s: i, s: i, s: o, s: o}", "ssrc", payload->report->ssrc, "type", payload->report->type, "report_count", payload->report->reception_report_count, - "sender_information", json_rtcp_sender_info ? json_rtcp_sender_info : ast_json_null(), - "report_blocks", json_rtcp_report_blocks); + "sender_information", json_rtcp_sender_info ? ast_json_ref(json_rtcp_sender_info) : ast_json_ref(ast_json_null()), + "report_blocks", ast_json_ref(json_rtcp_report_blocks)); if (!json_rtcp_report) { return NULL; } @@ -2025,10 +2025,10 @@ static struct ast_json *rtcp_report_to_json(struct stasis_message *msg, } } - return ast_json_pack("{s: O, s: O, s: O}", - "channel", payload->snapshot ? json_channel : ast_json_null(), - "rtcp_report", json_rtcp_report, - "blob", payload->blob); + return ast_json_pack("{s: o, s: o, s: o}", + "channel", payload->snapshot ? ast_json_ref(json_channel) : ast_json_ref(ast_json_null()), + "rtcp_report", ast_json_ref(json_rtcp_report), + "blob", ast_json_deep_copy(payload->blob)); } static void rtp_rtcp_report_dtor(void *obj) diff --git a/main/stasis.c b/main/stasis.c index 6adbfc3b69..962efc83ed 100644 --- a/main/stasis.c +++ b/main/stasis.c @@ -1274,8 +1274,8 @@ static struct ast_json *multi_user_event_to_json( ast_json_object_set(out, "type", ast_json_string_create("ChannelUserevent")); ast_json_object_set(out, "timestamp", ast_json_timeval(*tv, NULL)); - ast_json_object_set(out, "eventname", ast_json_ref(ast_json_object_get(blob, "eventname"))); - ast_json_object_set(out, "userevent", ast_json_ref(blob)); /* eventname gets duplicated, that's ok */ + ast_json_object_set(out, "eventname", ast_json_string_create(ast_json_string_get((ast_json_object_get(blob, "eventname"))))); + ast_json_object_set(out, "userevent", ast_json_deep_copy(blob)); for (type = 0; type < STASIS_UMOS_MAX; ++type) { for (i = 0; i < AST_VECTOR_SIZE(&multi->snapshots[type]); ++i) { diff --git a/main/stasis_channels.c b/main/stasis_channels.c index b8efd407b5..d46a8ddcf0 100644 --- a/main/stasis_channels.c +++ b/main/stasis_channels.c @@ -1016,6 +1016,10 @@ static struct ast_json *dtmf_end_to_json( struct ast_channel_snapshot *snapshot = channel_blob->snapshot; const char *direction = ast_json_string_get(ast_json_object_get(blob, "direction")); + const char *digit = + ast_json_string_get(ast_json_object_get(blob, "digit")); + long duration_ms = + ast_json_integer_get(ast_json_object_get(blob, "duration_ms")); const struct timeval *tv = stasis_message_timestamp(message); struct ast_json *json_channel; @@ -1029,11 +1033,11 @@ static struct ast_json *dtmf_end_to_json( return NULL; } - return ast_json_pack("{s: s, s: o, s: O, s: O, s: o}", + return ast_json_pack("{s: s, s: o, s: s, s: i, s: o}", "type", "ChannelDtmfReceived", "timestamp", ast_json_timeval(*tv, NULL), - "digit", ast_json_object_get(blob, "digit"), - "duration_ms", ast_json_object_get(blob, "duration_ms"), + "digit", digit, + "duration_ms", duration_ms, "channel", json_channel); } @@ -1057,6 +1061,12 @@ static struct ast_json *dial_to_json( { struct ast_multi_channel_blob *payload = stasis_message_data(message); struct ast_json *blob = ast_multi_channel_blob_get_json(payload); + const char *dialstatus = + ast_json_string_get(ast_json_object_get(blob, "dialstatus")); + const char *forward = + ast_json_string_get(ast_json_object_get(blob, "forward")); + const char *dialstring = + ast_json_string_get(ast_json_object_get(blob, "dialstring")); struct ast_json *caller_json = ast_channel_snapshot_to_json(ast_multi_channel_blob_get_channel(payload, "caller"), sanitize); struct ast_json *peer_json = ast_channel_snapshot_to_json(ast_multi_channel_blob_get_channel(payload, "peer"), sanitize); struct ast_json *forwarded_json = ast_channel_snapshot_to_json(ast_multi_channel_blob_get_channel(payload, "forwarded"), sanitize); @@ -1064,12 +1074,12 @@ static struct ast_json *dial_to_json( const struct timeval *tv = stasis_message_timestamp(message); int res = 0; - json = ast_json_pack("{s: s, s: o, s: O, s: O, s: O}", + json = ast_json_pack("{s: s, s: o, s: s, s: s, s: s}", "type", "Dial", "timestamp", ast_json_timeval(*tv, NULL), - "dialstatus", ast_json_object_get(blob, "dialstatus"), - "forward", ast_json_object_get(blob, "forward"), - "dialstring", ast_json_object_get(blob, "dialstring")); + "dialstatus", dialstatus, + "forward", forward, + "dialstring", dialstring); if (!json) { ast_json_unref(caller_json); ast_json_unref(peer_json); diff --git a/res/res_fax.c b/res/res_fax.c index e947c91c18..ef0e27696a 100644 --- a/res/res_fax.c +++ b/res/res_fax.c @@ -2028,14 +2028,14 @@ static int report_receive_fax_status(struct ast_channel *chan, const char *filen fax_bitrate = ast_strdupa(fax_bitrate); } - json_object = ast_json_pack("{s: s, s: s, s: s, s: s, s: s, s: s, s: O}", + json_object = ast_json_pack("{s: s, s: s, s: s, s: s, s: s, s: s, s: o}", "type", "receive", "remote_station_id", S_OR(remote_station_id, ""), "local_station_id", S_OR(local_station_id, ""), "fax_pages", S_OR(fax_pages, ""), "fax_resolution", S_OR(fax_resolution, ""), "fax_bitrate", S_OR(fax_bitrate, ""), - "filenames", json_array); + "filenames", ast_json_ref(json_array)); if (!json_object) { return -1; } diff --git a/res/res_stasis.c b/res/res_stasis.c index 1cab8c389b..94b037e69c 100644 --- a/res/res_stasis.c +++ b/res/res_stasis.c @@ -150,10 +150,10 @@ static struct ast_json *stasis_start_to_json(struct stasis_message *message, return NULL; } - msg = ast_json_pack("{s: s, s: O, s: O, s: o}", + msg = ast_json_pack("{s: s, s: o, s: o, s: o}", "type", "StasisStart", - "timestamp", ast_json_object_get(payload->blob, "timestamp"), - "args", ast_json_object_get(payload->blob, "args"), + "timestamp", ast_json_copy(ast_json_object_get(payload->blob, "timestamp")), + "args", ast_json_deep_copy(ast_json_object_get(payload->blob, "args")), "channel", ast_channel_snapshot_to_json(payload->channel, NULL)); if (!msg) { ast_log(LOG_ERROR, "Failed to pack JSON for StasisStart message\n"); diff --git a/res/res_stasis_playback.c b/res/res_stasis_playback.c index dc6e8f2b8b..779dd77550 100644 --- a/res/res_stasis_playback.c +++ b/res/res_stasis_playback.c @@ -105,9 +105,9 @@ static struct ast_json *playback_to_json(struct stasis_message *message, return NULL; } - return ast_json_pack("{s: s, s: O}", + return ast_json_pack("{s: s, s: o}", "type", type, - "playback", blob); + "playback", ast_json_deep_copy(blob)); } STASIS_MESSAGE_TYPE_DEFN(stasis_app_playback_snapshot_type, diff --git a/res/res_stasis_recording.c b/res/res_stasis_recording.c index 50e0697b18..392d92c8e5 100644 --- a/res/res_stasis_recording.c +++ b/res/res_stasis_recording.c @@ -91,9 +91,9 @@ static struct ast_json *recording_to_json(struct stasis_message *message, return NULL; } - return ast_json_pack("{s: s, s: O}", + return ast_json_pack("{s: s, s: o}", "type", type, - "recording", blob); + "recording", ast_json_deep_copy(blob)); } STASIS_MESSAGE_TYPE_DEFN(stasis_app_recording_snapshot_type, diff --git a/res/stasis/app.c b/res/stasis/app.c index 3539182410..4e18aa5ae7 100644 --- a/res/stasis/app.c +++ b/res/stasis/app.c @@ -610,11 +610,11 @@ static int message_received_handler(const char *endpoint_id, struct ast_json *js return -1; } - app_send(app, ast_json_pack("{s: s, s: o, s: o, s: O}", + app_send(app, ast_json_pack("{s: s, s: o, s: o, s: o}", "type", "TextMessageReceived", "timestamp", ast_json_timeval(ast_tvnow(), NULL), "endpoint", json_endpoint, - "message", json_msg)); + "message", ast_json_ref(json_msg))); return 0; }