From a2da1de749bc35c7ba9b777cb332c1b0999fa02b Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Mon, 22 Jul 2024 13:15:56 +0300
Subject: [PATCH] tuple: allocate formats table statically
The tuple formats table may be accessed with `tuple_format_by_id()` from
any thread, not just tx. For example, it's accessed by a vinyl writer
thread when it deletes a tuple. If a thread happens to access the table
while it's being reallocated by tx, see `tuple_format_register()`,
the accessing thread may crash with a use-after-free or NULL pointer
dereference bug, like the one below:
```
# 1 0x64bd45c09e22 in crash_signal_cb+162
# 2 0x76ce74e45320 in __sigaction+80
# 3 0x64bd45ab070c in vy_run_writer_append_stmt+700
# 4 0x64bd45ada32a in vy_task_write_run+234
# 5 0x64bd45ad84fe in vy_task_f+46
# 6 0x64bd45a4aba0 in fiber_cxx_invoke(int (*)(__va_list_tag*), __va_list_tag*)+16
# 7 0x64bd45c13e66 in fiber_loop+70
# 8 0x64bd45e83b9c in coro_init+76
```
To avoid that, let's make the tuple formats table statically allocated.
This shouldn't increase actual memory usage because system memory is
allocated lazily, on page fault. The max number of tuple formats isn't
that big (64K) to care about the increase in virtual memory usage.
Closes #10278
NO_DOC=bug fix
NO_TEST=mt race
---
.../gh-10278-vy-tuple-format-lookup-fix.md | 4 +++
src/box/tuple_format.c | 27 +++----------------
src/box/tuple_format.h | 2 +-
test/wal_off/alter.result | 2 +-
4 files changed, 9 insertions(+), 26 deletions(-)
create mode 100644 changelogs/unreleased/gh-10278-vy-tuple-format-lookup-fix.md
diff --git a/changelogs/unreleased/gh-10278-vy-tuple-format-lookup-fix.md b/changelogs/unreleased/gh-10278-vy-tuple-format-lookup-fix.md
new file mode 100644
index 0000000000..fd2a6322f8
--- /dev/null
+++ b/changelogs/unreleased/gh-10278-vy-tuple-format-lookup-fix.md
@@ -0,0 +1,4 @@
+## bugfix/vinyl
+
+* Fixed a multi-threading race condition that could cause a writer thread to
+ crash while looking up a tuple format (gh-10278).
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 54d6ededac..36dbdf96ad 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -44,10 +44,10 @@
#include <PMurHash.h>
/** Global table of tuple formats */
-struct tuple_format **tuple_formats;
+struct tuple_format *tuple_formats[FORMAT_ID_MAX + 1];
static intptr_t recycled_format_ids = FORMAT_ID_NIL;
-static uint32_t formats_size = 0, formats_capacity = 0;
+static uint32_t formats_size = 0;
static uint64_t formats_epoch = 0;
/**
@@ -660,27 +660,9 @@ static int
tuple_format_register(struct tuple_format *format)
{
if (recycled_format_ids != FORMAT_ID_NIL) {
-
format->id = (uint16_t) recycled_format_ids;
recycled_format_ids = (intptr_t) tuple_formats[recycled_format_ids];
} else {
- if (formats_size == formats_capacity) {
- uint32_t new_capacity = formats_capacity ?
- formats_capacity * 2 : 16;
- struct tuple_format **formats;
- formats = (struct tuple_format **)
- realloc(tuple_formats, new_capacity *
- sizeof(tuple_formats[0]));
- if (formats == NULL) {
- diag_set(OutOfMemory,
- sizeof(struct tuple_format), "malloc",
- "tuple_formats");
- return -1;
- }
-
- formats_capacity = new_capacity;
- tuple_formats = formats;
- }
uint32_t formats_size_max = FORMAT_ID_MAX + 1;
struct errinj *inj = errinj(ERRINJ_TUPLE_FORMAT_COUNT,
ERRINJ_INT);
@@ -688,7 +670,7 @@ tuple_format_register(struct tuple_format *format)
formats_size_max = inj->iparam;
if (formats_size >= formats_size_max) {
diag_set(ClientError, ER_TUPLE_FORMAT_LIMIT,
- (unsigned) formats_capacity);
+ (unsigned)formats_size_max);
return -1;
}
format->id = formats_size++;
@@ -1290,10 +1272,8 @@ void
tuple_format_init()
{
tuple_formats_hash = mh_tuple_format_new();
- tuple_formats = NULL;
recycled_format_ids = FORMAT_ID_NIL;
formats_size = 0;
- formats_capacity = 0;
}
/** Destroy tuple format subsystem and free resourses */
@@ -1314,7 +1294,6 @@ tuple_format_free()
free(*format);
}
}
- free(tuple_formats);
mh_tuple_format_delete(tuple_formats_hash);
}
diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
index 81eaa4f77a..b006dc3d60 100644
--- a/src/box/tuple_format.h
+++ b/src/box/tuple_format.h
@@ -373,7 +373,7 @@ tuple_format_field(struct tuple_format *format, uint32_t fieldno)
return tuple_format_field_by_path(format, fieldno, NULL, 0, 0);
}
-extern struct tuple_format **tuple_formats;
+extern struct tuple_format *tuple_formats[];
static inline uint32_t
tuple_format_id(struct tuple_format *format)
diff --git a/test/wal_off/alter.result b/test/wal_off/alter.result
index 042ca67bca..c0aff9502f 100644
--- a/test/wal_off/alter.result
+++ b/test/wal_off/alter.result
@@ -24,7 +24,7 @@ for k = 1, box.schema.FORMAT_ID_MAX, 1 do
table.insert(spaces, s)
end;
---
-- error: 'Tuple format limit reached: 65536'
+- error: 'Tuple format limit reached: 65535'
...
#spaces > 65000;
---
--
GitLab