From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: tarantool-patches@dev.tarantool.org, gorcunov@gmail.com, sergepetrenko@tarantool.org Subject: [Tarantool-patches] [PATCH 1/2] swim: fix crash on bad member_by_uuid() call Date: Sun, 4 Apr 2021 17:06:42 +0200 [thread overview] Message-ID: <ee347bd2d6e5f0b84ef65883d92aac8c0c7e6ba0.1617548717.git.v.shpilevoy@tarantool.org> (raw) In-Reply-To: <cover.1617548717.git.v.shpilevoy@tarantool.org> In Lua swim object's method member_by_uuid() could crash if called with no arguments. UUID was then passed as NULL, and dereferenced. The patch makes member_by_uuid() treat NULL like nil UUID and return NULL (member not found). The reason is that swim_member_by_uuid() can't fail. It can only return a member or not. It never sets a diag error. Closes #5951 --- .../unreleased/swim-member_by_uuid-crash.md | 4 ++++ src/lib/swim/swim.c | 2 ++ test/swim/swim.result | 9 ++++++++ test/swim/swim.test.lua | 3 +++ test/unit/swim.c | 23 ++++++++++++++++++- test/unit/swim.result | 9 +++++++- 6 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/swim-member_by_uuid-crash.md diff --git a/changelogs/unreleased/swim-member_by_uuid-crash.md b/changelogs/unreleased/swim-member_by_uuid-crash.md new file mode 100644 index 000000000..d710aaa73 --- /dev/null +++ b/changelogs/unreleased/swim-member_by_uuid-crash.md @@ -0,0 +1,4 @@ +## bugfix/swim + +* Fix the crash on an attempt to call `swim:member_by_uuid()` with no arguments + or with `nil`/`box.NULL` (gh-5951). diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c index 9b5458635..1ecc90414 100644 --- a/src/lib/swim/swim.c +++ b/src/lib/swim/swim.c @@ -2337,6 +2337,8 @@ struct swim_member * swim_member_by_uuid(struct swim *swim, const struct tt_uuid *uuid) { assert(swim_is_configured(swim)); + if (uuid == NULL) + return NULL; return swim_find_member(swim, uuid); } diff --git a/test/swim/swim.result b/test/swim/swim.result index bfc83c143..bab8d20eb 100644 --- a/test/swim/swim.result +++ b/test/swim/swim.result @@ -487,6 +487,15 @@ s1:member_by_uuid(50) - error: 'builtin/swim.lua:<line>: swim:member_by_uuid: expected string UUID or struct tt_uuid' ... +-- gh-5951: could crash with no arguments or NULL. +s1:member_by_uuid() +--- +- null +... +s1:member_by_uuid(box.NULL) +--- +- null +... s1:member_by_uuid(uuid(2)) --- - null diff --git a/test/swim/swim.test.lua b/test/swim/swim.test.lua index 1593d8833..bec8b9cca 100644 --- a/test/swim/swim.test.lua +++ b/test/swim/swim.test.lua @@ -159,6 +159,9 @@ s.is_dropped() s1:member_by_uuid(uuid(1)) ~= nil s1:member_by_uuid(50) +-- gh-5951: could crash with no arguments or NULL. +s1:member_by_uuid() +s1:member_by_uuid(box.NULL) s1:member_by_uuid(uuid(2)) -- UUID can be cdata. diff --git a/test/unit/swim.c b/test/unit/swim.c index a506d04e9..a495f9648 100644 --- a/test/unit/swim.c +++ b/test/unit/swim.c @@ -1071,10 +1071,30 @@ swim_test_suspect_new_members(void) swim_finish_test(); } +static void +swim_test_member_by_uuid(void) +{ + swim_start_test(3); + struct swim_cluster *cluster = swim_cluster_new(1); + + struct swim *s1 = swim_cluster_member(cluster, 0); + const struct swim_member *s1_self = swim_self(s1); + is(swim_member_by_uuid(s1, swim_member_uuid(s1_self)), s1_self, + "found by UUID") + + struct tt_uuid uuid = uuid_nil; + uuid.time_low = 1000; + is(swim_member_by_uuid(s1, &uuid), NULL, "not found by valid UUID"); + is(swim_member_by_uuid(s1, NULL), NULL, "not found by NULL UUID"); + + swim_cluster_delete(cluster); + swim_finish_test(); +} + static int main_f(va_list ap) { - swim_start_test(22); + swim_start_test(23); (void) ap; fakeev_init(); @@ -1102,6 +1122,7 @@ main_f(va_list ap) swim_test_generation(); swim_test_dissemination_speed(); swim_test_suspect_new_members(); + swim_test_member_by_uuid(); fakenet_free(); fakeev_free(); diff --git a/test/unit/swim.result b/test/unit/swim.result index d4b1dba49..e1af4d616 100644 --- a/test/unit/swim.result +++ b/test/unit/swim.result @@ -1,5 +1,5 @@ *** main_f *** -1..22 +1..23 *** swim_test_one_link *** 1..6 ok 1 - no rounds - no fullmesh @@ -225,4 +225,11 @@ ok 21 - subtests ok 2 - S3 didn't add S1 from S2's messages, because S1 didn't answer on a ping ok 22 - subtests *** swim_test_suspect_new_members: done *** + *** swim_test_member_by_uuid *** + 1..3 + ok 1 - found by UUID + ok 2 - not found by valid UUID + ok 3 - not found by NULL UUID +ok 23 - subtests + *** swim_test_member_by_uuid: done *** *** main_f: done *** -- 2.24.3 (Apple Git-128)
next prev parent reply other threads:[~2021-04-04 15:07 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-04 15:06 [Tarantool-patches] [PATCH 0/2] Swim misusage crashes Vladislav Shpilevoy via Tarantool-patches 2021-04-04 15:06 ` Vladislav Shpilevoy via Tarantool-patches [this message] 2021-04-04 15:06 ` [Tarantool-patches] [PATCH 2/2] swim: check types in __serialize methods Vladislav Shpilevoy via Tarantool-patches 2021-04-04 20:42 ` [Tarantool-patches] [PATCH 0/2] Swim misusage crashes Cyrill Gorcunov via Tarantool-patches 2021-04-05 9:05 ` Serge Petrenko via Tarantool-patches 2021-04-05 15:20 ` Kirill Yukhin via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ee347bd2d6e5f0b84ef65883d92aac8c0c7e6ba0.1617548717.git.v.shpilevoy@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=gorcunov@gmail.com \ --cc=sergepetrenko@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 1/2] swim: fix crash on bad member_by_uuid() call' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox