Skip to content

Commit

Permalink
json_config: do not assume JSON contains 'params' field
Browse files Browse the repository at this point in the history
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 <[email protected]>
Change-Id: I7ef1a764467817ad788fdf5dbe17eaeb99dcc22e
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3256
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <[email protected]>
Reviewed-by: Jim Harris <[email protected]>
Reviewed-by: Shuhei Matsumoto <[email protected]>
  • Loading branch information
tomzawadzki committed Jul 31, 2020
1 parent 0d8f86f commit cebf307
Showing 1 changed file with 16 additions and 10 deletions.
26 changes: 16 additions & 10 deletions lib/event/json_config.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
SPDK_DEBUG_APP_CFG("\tmethod: %s\n", cfg.method);

/* Need to add one character to include '}' */
params_len = params_end->start - cfg.params->start + 1;
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;

SPDK_DEBUG_APP_CFG("\tmethod: %s\n", cfg.method);
SPDK_DEBUG_APP_CFG("\tparams: %.*s\n", (int)params_len, (char *)cfg.params->start);
/* 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) {
Expand All @@ -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);
Expand Down

0 comments on commit cebf307

Please sign in to comment.