From bc0daf99b35bff9f6a4afa85b1e7faabcbc68144 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Thu, 6 Jun 2024 10:56:54 +0300
Subject: [PATCH] tuple: fix crash on hashing tuple with double fields
`tuple_hash_field()` doesn't advance the MsgPack cursor after hashing
a tuple field with the type `double`, which can result in crashes both
in memtx (while inserting a tuple into a hash index) and in vinyl
(while writing a bloom filter on dump or compaction).
The bug was introduced by commit 51af059c10ff ("box: compare and hash
msgpack value of double key field as double").
Closes #10090
NO_DOC=bug fix
---
.../gh-10090-tuple-hash-double-fix.md | 6 +++
src/box/tuple_hash.cc | 2 +-
.../gh_10090_tuple_hash_double_test.lua | 37 ++++++++++++++++++
.../gh_10090_tuple_hash_double_test.lua | 38 +++++++++++++++++++
4 files changed, 82 insertions(+), 1 deletion(-)
create mode 100644 changelogs/unreleased/gh-10090-tuple-hash-double-fix.md
create mode 100644 test/box-luatest/gh_10090_tuple_hash_double_test.lua
create mode 100644 test/vinyl-luatest/gh_10090_tuple_hash_double_test.lua
diff --git a/changelogs/unreleased/gh-10090-tuple-hash-double-fix.md b/changelogs/unreleased/gh-10090-tuple-hash-double-fix.md
new file mode 100644
index 0000000000..7d4023565a
--- /dev/null
+++ b/changelogs/unreleased/gh-10090-tuple-hash-double-fix.md
@@ -0,0 +1,6 @@
+## bugfix/core
+
+* Fixed a bug when hashing a tuple with `double` fields could crash.
+ The bug could trigger a crash in memtx while inserting a tuple into
+ a `hash` index and in vinyl while writing a bloom filter on dump or
+ compaction (gh-10090).
diff --git a/src/box/tuple_hash.cc b/src/box/tuple_hash.cc
index 1c921dda12..4e8e2bac45 100644
--- a/src/box/tuple_hash.cc
+++ b/src/box/tuple_hash.cc
@@ -313,7 +313,7 @@ tuple_hash_field(uint32_t *ph1, uint32_t *pcarry, const char **field,
* This will only fail if the mp_type is not numeric, which is
* impossible here (see field_mp_plain_type_is_compatible).
*/
- if (mp_read_double_lossy(&f, &value) == -1)
+ if (mp_read_double_lossy(field, &value) == -1)
unreachable();
char *double_msgpack_end = mp_encode_double(buf, value);
size = double_msgpack_end - buf;
diff --git a/test/box-luatest/gh_10090_tuple_hash_double_test.lua b/test/box-luatest/gh_10090_tuple_hash_double_test.lua
new file mode 100644
index 0000000000..141587be99
--- /dev/null
+++ b/test/box-luatest/gh_10090_tuple_hash_double_test.lua
@@ -0,0 +1,37 @@
+local server = require('luatest.server')
+local t = require('luatest')
+
+local g = t.group()
+
+g.before_all(function(cg)
+ cg.server = server:new()
+ cg.server:start()
+end)
+
+g.after_all(function(cg)
+ cg.server:drop()
+end)
+
+g.after_each(function(cg)
+ cg.server:exec(function()
+ if box.space.test ~= nil then
+ box.space.test:drop()
+ end
+ end)
+end)
+
+g.test_tuple_hash_double = function(cg)
+ cg.server:exec(function()
+ local s = box.schema.create_space('test')
+ s:create_index('pk', {
+ type = 'hash',
+ parts = {
+ {1, 'double'}, {2, 'string'}, {3, 'double'},
+ },
+ })
+ s:insert{1, 'x', 0.5}
+ s:insert{0.5, 'y', 1}
+ s:delete{0.5, 'y', 1}
+ t.assert_equals(s:select({}, {fullscan = true}), {{1, 'x', 0.5}})
+ end)
+end
diff --git a/test/vinyl-luatest/gh_10090_tuple_hash_double_test.lua b/test/vinyl-luatest/gh_10090_tuple_hash_double_test.lua
new file mode 100644
index 0000000000..f75a83864c
--- /dev/null
+++ b/test/vinyl-luatest/gh_10090_tuple_hash_double_test.lua
@@ -0,0 +1,38 @@
+local server = require('luatest.server')
+local t = require('luatest')
+
+local g = t.group()
+
+g.before_all(function(cg)
+ cg.server = server:new()
+ cg.server:start()
+end)
+
+g.after_all(function(cg)
+ cg.server:drop()
+end)
+
+g.after_each(function(cg)
+ cg.server:exec(function()
+ if box.space.test ~= nil then
+ box.space.test:drop()
+ end
+ end)
+end)
+
+g.test_tuple_hash_double = function(cg)
+ cg.server:exec(function()
+ local s = box.schema.create_space('test', {engine = 'vinyl'})
+ s:create_index('pk', {
+ parts = {
+ {1, 'double'}, {2, 'string'}, {3, 'double'},
+ },
+ })
+ s:insert{1, 'x', 0.5}
+ s:insert{0.5, 'y', 1}
+ box.snapshot()
+ s:delete{0.5, 'y', 1}
+ box.snapshot()
+ t.assert_equals(s:select({}, {fullscan = true}), {{1, 'x', 0.5}})
+ end)
+end
--
GitLab