Skip to content

Commit d588bb3

Browse files
VoletiRamRam Prasad Voleti
and
Ram Prasad Voleti
authored
Fix raxRemove crash at memcpy() due to key size exceeds max Rax size (#1722)
Fix raxRemove crash at memcpy() (line 1181) due to key size exceeds `RAX_NODE_MAX_SIZE`. Note that this could happen when key size was more than 512MB if we allow it by increasing the default `proto-max-bulk-len`. The crash could happen when we recompress the rax after removing a key due to expiry or DEL while memcpy() merge the key that exceed 512MB limit. While the counting phase has the size check, the actual compress logic is missing it which lead to this crash. --------- Signed-off-by: Ram Prasad Voleti <[email protected]> Co-authored-by: Ram Prasad Voleti <[email protected]>
1 parent e6e65c6 commit d588bb3

File tree

3 files changed

+55
-1
lines changed

3 files changed

+55
-1
lines changed

Diff for: src/rax.c

+1
Original file line numberDiff line numberDiff line change
@@ -1187,6 +1187,7 @@ int raxRemove(rax *rax, unsigned char *s, size_t len, void **old) {
11871187
rax_free(tofree);
11881188
rax->numnodes--;
11891189
if (h->iskey || (!h->iscompr && h->size != 1)) break;
1190+
if (comprsize + h->size > RAX_NODE_MAX_SIZE) break;
11901191
}
11911192
debugnode("New node", new);
11921193

Diff for: src/unit/test_files.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ int test_raxRegressionTest6(int argc, char **argv, int flags);
175175
int test_raxBenchmark(int argc, char **argv, int flags);
176176
int test_raxHugeKey(int argc, char **argv, int flags);
177177
int test_raxFuzz(int argc, char **argv, int flags);
178+
int test_raxRecompressHugeKey(int argc, char **argv, int flags);
178179
int test_sds(int argc, char **argv, int flags);
179180
int test_typesAndAllocSize(int argc, char **argv, int flags);
180181
int test_sdsHeaderSizes(int argc, char **argv, int flags);
@@ -241,7 +242,7 @@ unitTest __test_listpack_c[] = {{"test_listpackCreateIntList", test_listpackCrea
241242
unitTest __test_networking_c[] = {{"test_writeToReplica", test_writeToReplica}, {"test_postWriteToReplica", test_postWriteToReplica}, {"test_backupAndUpdateClientArgv", test_backupAndUpdateClientArgv}, {"test_rewriteClientCommandArgument", test_rewriteClientCommandArgument}, {NULL, NULL}};
242243
unitTest __test_object_c[] = {{"test_object_with_key", test_object_with_key}, {NULL, NULL}};
243244
unitTest __test_quicklist_c[] = {{"test_quicklistCreateList", test_quicklistCreateList}, {"test_quicklistAddToTailOfEmptyList", test_quicklistAddToTailOfEmptyList}, {"test_quicklistAddToHeadOfEmptyList", test_quicklistAddToHeadOfEmptyList}, {"test_quicklistAddToTail5xAtCompress", test_quicklistAddToTail5xAtCompress}, {"test_quicklistAddToHead5xAtCompress", test_quicklistAddToHead5xAtCompress}, {"test_quicklistAddToTail500xAtCompress", test_quicklistAddToTail500xAtCompress}, {"test_quicklistAddToHead500xAtCompress", test_quicklistAddToHead500xAtCompress}, {"test_quicklistRotateEmpty", test_quicklistRotateEmpty}, {"test_quicklistComprassionPlainNode", test_quicklistComprassionPlainNode}, {"test_quicklistNextPlainNode", test_quicklistNextPlainNode}, {"test_quicklistRotatePlainNode", test_quicklistRotatePlainNode}, {"test_quicklistRotateOneValOnce", test_quicklistRotateOneValOnce}, {"test_quicklistRotate500Val5000TimesAtCompress", test_quicklistRotate500Val5000TimesAtCompress}, {"test_quicklistPopEmpty", test_quicklistPopEmpty}, {"test_quicklistPop1StringFrom1", test_quicklistPop1StringFrom1}, {"test_quicklistPopHead1NumberFrom1", test_quicklistPopHead1NumberFrom1}, {"test_quicklistPopHead500From500", test_quicklistPopHead500From500}, {"test_quicklistPopHead5000From500", test_quicklistPopHead5000From500}, {"test_quicklistIterateForwardOver500List", test_quicklistIterateForwardOver500List}, {"test_quicklistIterateReverseOver500List", test_quicklistIterateReverseOver500List}, {"test_quicklistInsertAfter1Element", test_quicklistInsertAfter1Element}, {"test_quicklistInsertBefore1Element", test_quicklistInsertBefore1Element}, {"test_quicklistInsertHeadWhileHeadNodeIsFull", test_quicklistInsertHeadWhileHeadNodeIsFull}, {"test_quicklistInsertTailWhileTailNodeIsFull", test_quicklistInsertTailWhileTailNodeIsFull}, {"test_quicklistInsertOnceInElementsWhileIteratingAtCompress", test_quicklistInsertOnceInElementsWhileIteratingAtCompress}, {"test_quicklistInsertBefore250NewInMiddleOf500ElementsAtCompress", test_quicklistInsertBefore250NewInMiddleOf500ElementsAtCompress}, {"test_quicklistInsertAfter250NewInMiddleOf500ElementsAtCompress", test_quicklistInsertAfter250NewInMiddleOf500ElementsAtCompress}, {"test_quicklistDuplicateEmptyList", test_quicklistDuplicateEmptyList}, {"test_quicklistDuplicateListOf1Element", test_quicklistDuplicateListOf1Element}, {"test_quicklistDuplicateListOf500", test_quicklistDuplicateListOf500}, {"test_quicklistIndex1200From500ListAtFill", test_quicklistIndex1200From500ListAtFill}, {"test_quicklistIndex12From500ListAtFill", test_quicklistIndex12From500ListAtFill}, {"test_quicklistIndex100From500ListAtFill", test_quicklistIndex100From500ListAtFill}, {"test_quicklistIndexTooBig1From50ListAtFill", test_quicklistIndexTooBig1From50ListAtFill}, {"test_quicklistDeleteRangeEmptyList", test_quicklistDeleteRangeEmptyList}, {"test_quicklistDeleteRangeOfEntireNodeInListOfOneNode", test_quicklistDeleteRangeOfEntireNodeInListOfOneNode}, {"test_quicklistDeleteRangeOfEntireNodeWithOverflowCounts", test_quicklistDeleteRangeOfEntireNodeWithOverflowCounts}, {"test_quicklistDeleteMiddle100Of500List", test_quicklistDeleteMiddle100Of500List}, {"test_quicklistDeleteLessThanFillButAcrossNodes", test_quicklistDeleteLessThanFillButAcrossNodes}, {"test_quicklistDeleteNegative1From500List", test_quicklistDeleteNegative1From500List}, {"test_quicklistDeleteNegative1From500ListWithOverflowCounts", test_quicklistDeleteNegative1From500ListWithOverflowCounts}, {"test_quicklistDeleteNegative100From500List", test_quicklistDeleteNegative100From500List}, {"test_quicklistDelete10Count5From50List", test_quicklistDelete10Count5From50List}, {"test_quicklistNumbersOnlyListRead", test_quicklistNumbersOnlyListRead}, {"test_quicklistNumbersLargerListRead", test_quicklistNumbersLargerListRead}, {"test_quicklistNumbersLargerListReadB", test_quicklistNumbersLargerListReadB}, {"test_quicklistLremTestAtCompress", test_quicklistLremTestAtCompress}, {"test_quicklistIterateReverseDeleteAtCompress", test_quicklistIterateReverseDeleteAtCompress}, {"test_quicklistIteratorAtIndexTestAtCompress", test_quicklistIteratorAtIndexTestAtCompress}, {"test_quicklistLtrimTestAAtCompress", test_quicklistLtrimTestAAtCompress}, {"test_quicklistLtrimTestBAtCompress", test_quicklistLtrimTestBAtCompress}, {"test_quicklistLtrimTestCAtCompress", test_quicklistLtrimTestCAtCompress}, {"test_quicklistLtrimTestDAtCompress", test_quicklistLtrimTestDAtCompress}, {"test_quicklistVerifySpecificCompressionOfInteriorNodes", test_quicklistVerifySpecificCompressionOfInteriorNodes}, {"test_quicklistBookmarkGetUpdatedToNextItem", test_quicklistBookmarkGetUpdatedToNextItem}, {"test_quicklistBookmarkLimit", test_quicklistBookmarkLimit}, {"test_quicklistCompressAndDecompressQuicklistListpackNode", test_quicklistCompressAndDecompressQuicklistListpackNode}, {"test_quicklistCompressAndDecomressQuicklistPlainNodeLargeThanUINT32MAX", test_quicklistCompressAndDecomressQuicklistPlainNodeLargeThanUINT32MAX}, {NULL, NULL}};
244-
unitTest __test_rax_c[] = {{"test_raxRandomWalk", test_raxRandomWalk}, {"test_raxIteratorUnitTests", test_raxIteratorUnitTests}, {"test_raxTryInsertUnitTests", test_raxTryInsertUnitTests}, {"test_raxRegressionTest1", test_raxRegressionTest1}, {"test_raxRegressionTest2", test_raxRegressionTest2}, {"test_raxRegressionTest3", test_raxRegressionTest3}, {"test_raxRegressionTest4", test_raxRegressionTest4}, {"test_raxRegressionTest5", test_raxRegressionTest5}, {"test_raxRegressionTest6", test_raxRegressionTest6}, {"test_raxBenchmark", test_raxBenchmark}, {"test_raxHugeKey", test_raxHugeKey}, {"test_raxFuzz", test_raxFuzz}, {NULL, NULL}};
245+
unitTest __test_rax_c[] = {{"test_raxRandomWalk", test_raxRandomWalk}, {"test_raxIteratorUnitTests", test_raxIteratorUnitTests}, {"test_raxTryInsertUnitTests", test_raxTryInsertUnitTests}, {"test_raxRegressionTest1", test_raxRegressionTest1}, {"test_raxRegressionTest2", test_raxRegressionTest2}, {"test_raxRegressionTest3", test_raxRegressionTest3}, {"test_raxRegressionTest4", test_raxRegressionTest4}, {"test_raxRegressionTest5", test_raxRegressionTest5}, {"test_raxRegressionTest6", test_raxRegressionTest6}, {"test_raxBenchmark", test_raxBenchmark}, {"test_raxHugeKey", test_raxHugeKey}, {"test_raxFuzz", test_raxFuzz}, {"test_raxRecompressHugeKey", test_raxRecompressHugeKey}, {NULL, NULL}};
245246
unitTest __test_sds_c[] = {{"test_sds", test_sds}, {"test_typesAndAllocSize", test_typesAndAllocSize}, {"test_sdsHeaderSizes", test_sdsHeaderSizes}, {"test_sdssplitargs", test_sdssplitargs}, {NULL, NULL}};
246247
unitTest __test_sha1_c[] = {{"test_sha1", test_sha1}, {NULL, NULL}};
247248
unitTest __test_util_c[] = {{"test_string2ll", test_string2ll}, {"test_string2l", test_string2l}, {"test_ll2string", test_ll2string}, {"test_ld2string", test_ld2string}, {"test_fixedpoint_d2string", test_fixedpoint_d2string}, {"test_version2num", test_version2num}, {"test_reclaimFilePageCache", test_reclaimFilePageCache}, {NULL, NULL}};

Diff for: src/unit/test_rax.c

+52
Original file line numberDiff line numberDiff line change
@@ -1023,3 +1023,55 @@ int test_raxFuzz(int argc, char **argv, int flags) {
10231023
}
10241024
return !!errors;
10251025
}
1026+
1027+
/* This test verifies that raxRemove correctly handles compression when two keys
1028+
* share a common prefix. Upon deletion of one key, rax attempts to recompress
1029+
* the structure back to its original form for other key. Historically, there was
1030+
* a crash when deleting one key because rax would attempt to recompress the
1031+
* structure without checking the 512MB size limit.
1032+
*
1033+
* This test is disabled by default because it uses a lot of memory. */
1034+
int test_raxRecompressHugeKey(int argc, char **argv, int flags) {
1035+
UNUSED(argc);
1036+
UNUSED(argv);
1037+
1038+
if (!(flags & UNIT_TEST_LARGE_MEMORY)) return 0;
1039+
1040+
rax *rt = raxNew();
1041+
1042+
/* Insert small keys */
1043+
char small_key[32];
1044+
const char *small_prefix = ",({5oM}";
1045+
int i;
1046+
for (i = 1; i <= 20; i++) {
1047+
snprintf(small_key, sizeof(small_key), "%s%d", small_prefix, i);
1048+
size_t keylen = strlen(small_key);
1049+
raxInsert(rt, (unsigned char *)small_key, keylen,
1050+
(void *)(long)i, NULL);
1051+
}
1052+
1053+
/* Insert large key exceeding compressed node size limit */
1054+
size_t max_keylen = ((1 << 29) - 1) + 100; // Compressed node limit + overflow
1055+
const char *large_prefix = ",({ABC}";
1056+
unsigned char *large_key = zmalloc(max_keylen + strlen(large_prefix));
1057+
if (!large_key) {
1058+
fprintf(stderr, "Failed to allocate memory for large key\n");
1059+
raxFree(rt);
1060+
return 1;
1061+
}
1062+
1063+
memcpy(large_key, large_prefix, strlen(large_prefix));
1064+
memset(large_key + strlen(large_prefix), '1', max_keylen);
1065+
raxInsert(rt, large_key, max_keylen + strlen(large_prefix), NULL, NULL);
1066+
1067+
/* Remove small keys to trigger recompression crash in raxRemove() */
1068+
for (i = 20; i >= 1; i--) {
1069+
snprintf(small_key, sizeof(small_key), "%s%d", small_prefix, i);
1070+
size_t keylen = strlen(small_key);
1071+
raxRemove(rt, (unsigned char *)small_key, keylen, NULL);
1072+
}
1073+
1074+
zfree(large_key);
1075+
raxFree(rt);
1076+
return 0;
1077+
}

0 commit comments

Comments
 (0)