Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce correct JSON config format #491

Merged
merged 2 commits into from
Oct 18, 2022
Merged

Conversation

igaw
Copy link
Collaborator

@igaw igaw commented Sep 30, 2022

While working on the a related topic, I run into several crashes due to wrong formatted JSON.

The crashes happened inside the json-c library:

(gdb) bt
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x00007ffff7dbe893 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
#2  0x00007ffff7d6b846 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007ffff7d5481c in __GI_abort () at abort.c:79
#4  0x00007ffff7d5472b in __assert_fail_base (fmt=0x7ffff7edaa80 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x7ffff7f721f0 "json_object_get_type(jso) == json_type_array", 
    file=0x7ffff7f72048 "/home/abuild/rpmbuild/BUILD/json-c-json-c-0.16-20220414/json_object.c", line=1473, function=<optimized out>) at assert.c:92
#5  0x00007ffff7d63c46 in __GI___assert_fail (assertion=0x7ffff7f721f0 "json_object_get_type(jso) == json_type_array", 
    file=0x7ffff7f72048 "/home/abuild/rpmbuild/BUILD/json-c-json-c-0.16-20220414/json_object.c", line=1473, function=0x7ffff7f72510 <__PRETTY_FUNCTION__.7> "json_object_array_length") at assert.c:101
#6  0x00007ffff7f68b7a in json_object_array_length () from /lib64/libjson-c.so.5
#7  0x00007ffff7fafb2f in json_read_config (r=0x4f9c50, config_file=0x4a58f5 "/etc/nvme/config.json") at ../subprojects/libnvme/src/nvme/json.c:180
#8  0x00007ffff7fa8da4 in nvme_read_config (r=0x4f9c50, config_file=0x4a58f5 "/etc/nvme/config.json") at ../subprojects/libnvme/src/nvme/tree.c:162
#9  0x000000000040a06c in nvmf_discover (desc=0x4b0210 "Discover NVMeoF subsystems and connect to them", argc=1, argv=0x7fffffffe120, connect=true) at ../fabrics.c:738
#10 0x0000000000426da4 in connect_all_cmd (argc=1, argv=0x7fffffffe120, command=0x4f23c0 <connect_all_cmd_cmd>, plugin=0x4f2900 <builtin>) at ../nvme.c:8266
#11 0x000000000044b0df in handle_plugin (argc=1, argv=0x7fffffffe120, plugin=0x4f2900 <builtin>) at ../plugin.c:164
#12 0x0000000000426f6f in main (argc=2, argv=0x7fffffffe118) at ../nvme.c:8318

This is due the fact we sometimes blindly operate on the json object tree.

json-c's json_object_from_fd() doesn't set the JSON_TOKENER_STRICT
flag. This means the parser is more failure tolerant. Let's be strict
and enforce fully correctly formatted configuration.

Signed-off-by: Daniel Wagner <[email protected]>
Do not blindly assume the file starts with an array. This avoids a
crash in json-c.

Signed-off-by: Daniel Wagner <[email protected]>
@igaw
Copy link
Collaborator Author

igaw commented Sep 30, 2022

We might want to add some more type checks ontop of this.

@igaw
Copy link
Collaborator Author

igaw commented Sep 30, 2022

Ideally we would validate against the schema.

@igaw
Copy link
Collaborator Author

igaw commented Oct 18, 2022

Ideally we would validate against the schema.

After some discussion during ALPSS we should aim to move all the config reading parts to the fronted. There are all the data structs available to config data to the library.

@igaw igaw merged commit 86edb55 into linux-nvme:master Oct 18, 2022
@igaw igaw deleted the harden-json-parser branch October 18, 2022 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant