From cebf307b61e3a60f702736f75945735c59564a19 Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Wed, 8 Jul 2020 12:16:12 -0400 Subject: [PATCH] json_config: do not assume JSON contains 'params' field Not all JSON methods require 'params' field to be supplied. Verification of the JSON is done on server side in parse_single_request(). We should not attempt to process garbage values on correct JSON config file during app start. Segfault can be observed if following valid JSON config is supplied: { "method": "framework_wait_init" } Resulting in: json_config.c:388:13: runtime error: applying non-zero offset 18446744073709551600 to null pointer AddressSanitizer:DEADLYSIGNAL ================================================================= ==3386067==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x0000007260ff bp 0x7ffe6ea06890 sp 0x7ffe6ea067e0 T0) ==3386067==The signal is caused by a READ memory access. ==3386067==Hint: this fault was caused by a dereference of a high value address (see register values below). Dissassemble the provided pc to learn which register was used. #0 0x7260ff in app_json_config_load_subsystem_config_entry /home/tzawadzk/spdk/lib/event/json_config.c:391 #1 0x7cbb13 in msg_queue_run_batch /home/tzawadzk/spdk/lib/thread/thread.c:505 #2 0x7cd00a in thread_poll /home/tzawadzk/spdk/lib/thread/thread.c:581 #3 0x7cfe18 in spdk_thread_poll /home/tzawadzk/spdk/lib/thread/thread.c:689 #4 0x71d6ef in _reactor_run /home/tzawadzk/spdk/lib/event/reactor.c:326 #5 0x71eb00 in reactor_run /home/tzawadzk/spdk/lib/event/reactor.c:382 #6 0x71f911 in spdk_reactors_start /home/tzawadzk/spdk/lib/event/reactor.c:477 #7 0x718237 in spdk_app_start /home/tzawadzk/spdk/lib/event/app.c:691 #8 0x407e94 in main /home/tzawadzk/spdk/app/spdk_tgt/spdk_tgt.c:120 #9 0x7f0f2eef2041 in __libc_start_main ../csu/libc-start.c:308 #10 0x4079ad in _start (/home/tzawadzk/spdk/build/bin/spdk_tgt+0x4079ad) Signed-off-by: Tomasz Zawadzki Change-Id: I7ef1a764467817ad788fdf5dbe17eaeb99dcc22e Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3256 Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto --- lib/event/json_config.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/event/json_config.c b/lib/event/json_config.c index 69a95097a..79fc09e06 100644 --- a/lib/event/json_config.c +++ b/lib/event/json_config.c @@ -384,14 +384,17 @@ app_json_config_load_subsystem_config_entry(void *_ctx) goto out; } - /* Get _END by skipping params and going back by one element. */ - params_end = cfg.params + spdk_json_val_len(cfg.params) - 1; - - /* Need to add one character to include '}' */ - params_len = params_end->start - cfg.params->start + 1; - SPDK_DEBUG_APP_CFG("\tmethod: %s\n", cfg.method); - SPDK_DEBUG_APP_CFG("\tparams: %.*s\n", (int)params_len, (char *)cfg.params->start); + + if (cfg.params) { + /* Get _END by skipping params and going back by one element. */ + params_end = cfg.params + spdk_json_val_len(cfg.params) - 1; + + /* Need to add one character to include '}' */ + params_len = params_end->start - cfg.params->start + 1; + + SPDK_DEBUG_APP_CFG("\tparams: %.*s\n", (int)params_len, (char *)cfg.params->start); + } rpc_request = spdk_jsonrpc_client_create_request(); if (!rpc_request) { @@ -408,10 +411,13 @@ app_json_config_load_subsystem_config_entry(void *_ctx) spdk_json_write_named_string(w, "method", cfg.method); - /* No need to parse "params". Just dump the whole content of "params" - * directly into the request and let the remote side verify it. */ - spdk_json_write_name(w, "params"); - spdk_json_write_val_raw(w, cfg.params->start, params_len); + if (cfg.params) { + /* No need to parse "params". Just dump the whole content of "params" + * directly into the request and let the remote side verify it. */ + spdk_json_write_name(w, "params"); + spdk_json_write_val_raw(w, cfg.params->start, params_len); + } + spdk_jsonrpc_end_request(rpc_request, w); rc = client_send_request(ctx, rpc_request, app_json_config_load_subsystem_config_entry_next);