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.

Sign up for GitHub

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

Support sanitizer test and fix uaf #55

Merged
merged 4 commits into from
Sep 26, 2023
Merged

Support sanitizer test and fix uaf #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b85c4e8
Add sanitizer test
chenyang8094 Sep 21, 2023
49b6f91
Fix ci err
chenyang8094 Sep 21, 2023
014f2ff
Add asan test for sort mode
chenyang8094 Sep 21, 2023
51f6b04
Fix uaf
chenyang8094 Sep 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,84 @@ jobs:
run: |
cd build
lcov -c -d ./ -o cover.info
- uses: codecov/codecov-action@v1
with:
file: build/cover.info
token: ${{ secrets.CODECOV_TOKEN }}
verbose: true

test-sanitizer-address-scan-mode:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: clone and make redis
run: |
sudo apt-get install git
git clone https://github.com/redis/redis
cd redis
git checkout 7.0
make SANITIZER=address REDIS_CFLAGS='-Werror' BUILD_TLS=yes MALLOC=libc
- name: Install LCOV
run: |
sudo apt-get --assume-yes install lcov > /dev/null
- name: make tairhash
run: |
mkdir build
cd build
cmake ../ -DSANITIZER_MODE=address
make
- name: test
run: |
sudo apt-get install tcl8.6 tclx
work_path=$(pwd)
module_path=$work_path/lib
sed -e "s#your_path#$module_path#g" tests/tairhash.tcl > redis/tests/unit/type/tairhash.tcl
sed -i 's#unit/type/string#unit/type/tairhash#g' redis/tests/test_helper.tcl
cd redis
./runtest --stack-logging --single unit/type/tairhash
- name: lcov collection
run: |
cd build
lcov -c -d ./ -o cover.info
- uses: codecov/codecov-action@v1
with:
file: build/cover.info
token: ${{ secrets.CODECOV_TOKEN }}
verbose: true

test-sanitizer-address-sort-mode:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: clone and make redis
run: |
sudo apt-get install git
git clone https://github.com/redis/redis
cd redis
git checkout 7.0
make SANITIZER=address REDIS_CFLAGS='-Werror' BUILD_TLS=yes MALLOC=libc
- name: Install LCOV
run: |
sudo apt-get --assume-yes install lcov > /dev/null
- name: make tairhash
run: |
mkdir build
cd build
cmake ../ -DSANITIZER_MODE=address -DSORT_MODE=yes
make
- name: test
run: |
sudo apt-get install tcl8.6 tclx
work_path=$(pwd)
module_path=$work_path/lib
sed -e "s#your_path#$module_path#g" tests/tairhash.tcl > redis/tests/unit/type/tairhash.tcl
sed -i 's#unit/type/string#unit/type/tairhash#g' redis/tests/test_helper.tcl
cd redis
./runtest --stack-logging --single unit/type/tairhash
- name: lcov collection
run: |
cd build
lcov -c -d ./ -o cover.info
- uses: codecov/codecov-action@v1
with:
file: build/cover.info
Expand Down
14 changes: 14 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,20 @@ if (GCOV_MODE)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage")
endif()

if(SANITIZER_MODE MATCHES "address")
set(CMAKE_BUILD_TYPE "DEBUG")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O0 -fsanitize=address -fno-omit-frame-pointer")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O0 -fsanitize=address -fno-omit-frame-pointer")
elseif(SANITIZER_MODE MATCHES "undefined")
set(CMAKE_BUILD_TYPE "DEBUG")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O0 -fsanitize=undefined -fno-omit-frame-pointer")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O0 -fsanitize=undefined -fno-omit-frame-pointer")
elseif(SANITIZER_MODE MATCHES "thread")
set(CMAKE_BUILD_TYPE "DEBUG")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O0 -fsanitize=thread -fno-omit-frame-pointer")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O0 -fsanitize=thread -fno-omit-frame-pointer")
endif(SANITIZER_MODE MATCHES "address")

set(CMAKE_CXX_VISIBILITY_PRESET hidden)
set(CMAKE_C_VISIBILITY_PRESET hidden)

Expand Down
65 changes: 59 additions & 6 deletions src/scan_algorithm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ void activeExpire(RedisModuleCtx *ctx, int dbid, uint64_t keys_per_loop) {

RedisModuleString *key, *field;
RedisModuleKey *real_key;
int may_delkey = 0;

long long when, now;
unsigned long zsl_len;
Expand All @@ -79,8 +80,7 @@ void activeExpire(RedisModuleCtx *ctx, int dbid, uint64_t keys_per_loop) {
Module_Assert(RedisModule_CallReplyType(keys_reply) == REDISMODULE_REPLY_ARRAY);
size_t keynum = RedisModule_CallReplyLength(keys_reply);

int j;
for (j = 0; j < keynum; j++) {
for (int j = 0; j < keynum; j++) {
RedisModuleCallReply *key_reply = RedisModule_CallReplyArrayElement(keys_reply, j);
Module_Assert(RedisModule_CallReplyType(key_reply) == REDISMODULE_REPLY_STRING);
key = RedisModule_CreateStringFromCallReply(key_reply);
Expand All @@ -89,8 +89,10 @@ void activeExpire(RedisModuleCtx *ctx, int dbid, uint64_t keys_per_loop) {
return REDISMODULE_KEYTYPE_EMPTY here, so we must deal with it until after this
bugfix: https://github.com/redis/redis/commit/1833d008b3af8628835b5f082c5b4b1359557893 */
if (RedisModule_KeyType(real_key) == REDISMODULE_KEYTYPE_EMPTY) {
RedisModule_CloseKey(real_key);
continue;
}

if (RedisModule_ModuleTypeGetType(real_key) == TairHashType) {
tair_hash_obj = RedisModule_ModuleTypeGetValue(real_key);
if (tair_hash_obj->expire_index->length > 0) {
Expand All @@ -117,17 +119,28 @@ void activeExpire(RedisModuleCtx *ctx, int dbid, uint64_t keys_per_loop) {
m_listNode *node;
while ((node = listFirst(keys)) != NULL) {
key = listNodeValue(node);
may_delkey = 0;
real_key = RedisModule_OpenKey(ctx, key, REDISMODULE_READ | REDISMODULE_WRITE | REDISMODULE_OPEN_KEY_NOTOUCH);
int type = RedisModule_KeyType(real_key);
if (type != REDISMODULE_KEYTYPE_EMPTY) {
Module_Assert(RedisModule_ModuleTypeGetType(real_key) == TairHashType);
} else {
/* Note: redis scan may return dup key. */
m_listDelNode(keys, node);
RedisModule_CloseKey(real_key);
continue;
}

mstime_t key_ttl = RedisModule_GetExpire(real_key);
if (key_ttl != REDISMODULE_NO_EXPIRE && key_ttl < 1000) { //
may_delkey = 1;
}

tair_hash_obj = RedisModule_ModuleTypeGetValue(real_key);
if (dictSize(tair_hash_obj->hash) == 1) {
may_delkey = 1;
}
RedisModule_CloseKey(real_key);

zsl_len = tair_hash_obj->expire_index->length;
Module_Assert(zsl_len > 0);
Expand All @@ -140,15 +153,30 @@ void activeExpire(RedisModuleCtx *ctx, int dbid, uint64_t keys_per_loop) {
g_expire_algorithm.stat_active_expired_field[dbid]++;
start_index++;
expire_keys_per_loop--;
if (may_delkey) {
break;
}
} else {
break;
}
ln2 = ln2->level[0].forward;
}

// If the key happens to expire, it will be released in fieldExpireIfNeeded.
if (may_delkey) {
real_key = RedisModule_OpenKey(ctx, key, REDISMODULE_READ | REDISMODULE_OPEN_KEY_NOTOUCH);
int type = RedisModule_KeyType(real_key);
if (type == REDISMODULE_KEYTYPE_EMPTY) {
m_listDelNode(keys, node);
RedisModule_CloseKey(real_key);
continue;
}
RedisModule_CloseKey(real_key);
}

if (start_index) {
m_zslDeleteRangeByRank(tair_hash_obj->expire_index, 1, start_index);
delEmptyTairHashIfNeeded(ctx, real_key, key, tair_hash_obj);
delEmptyTairHashIfNeeded(ctx, NULL, key, tair_hash_obj);
}
m_listDelNode(keys, node);
}
Expand All @@ -164,6 +192,7 @@ void passiveExpire(RedisModuleCtx *ctx, int dbid, RedisModuleString *key) {
RedisModuleString *field;
RedisModuleKey *real_key;
unsigned long zsl_len;
int may_delkey = 0;
/* 1. The current db does not have a key that needs to expire. */
list *keys = m_listCreate();

Expand All @@ -186,6 +215,7 @@ void passiveExpire(RedisModuleCtx *ctx, int dbid, RedisModuleString *key) {
m_listNode *node;
while ((node = listFirst(keys)) != NULL) {
key = listNodeValue(node);
may_delkey = 0;
real_key = RedisModule_OpenKey(ctx, key, REDISMODULE_READ | REDISMODULE_WRITE);
int type = RedisModule_KeyType(real_key);

Expand All @@ -195,6 +225,17 @@ void passiveExpire(RedisModuleCtx *ctx, int dbid, RedisModuleString *key) {
zsl_len = tair_hash_obj->expire_index->length;
Module_Assert(zsl_len > 0);

mstime_t key_ttl = RedisModule_GetExpire(real_key);
if (key_ttl != REDISMODULE_NO_EXPIRE && key_ttl < 100) { // 100ms
may_delkey = 1;
}

tair_hash_obj = RedisModule_ModuleTypeGetValue(real_key);
if (dictSize(tair_hash_obj->hash) == 1) {
may_delkey = 1;
}
RedisModule_CloseKey(real_key);

start_index = 0;
ln = tair_hash_obj->expire_index->header->level[0].forward;
while (ln && keys_per_loop) {
Expand All @@ -203,17 +244,29 @@ void passiveExpire(RedisModuleCtx *ctx, int dbid, RedisModuleString *key) {
g_expire_algorithm.stat_passive_expired_field[dbid]++;
start_index++;
keys_per_loop--;
if (may_delkey) {
break;
}
} else {
break;
}
ln = ln->level[0].forward;
}

if (may_delkey) {
real_key = RedisModule_OpenKey(ctx, key, REDISMODULE_READ | REDISMODULE_OPEN_KEY_NOTOUCH);
int type = RedisModule_KeyType(real_key);
if (type == REDISMODULE_KEYTYPE_EMPTY) {
m_listDelNode(keys, node);
RedisModule_CloseKey(real_key);
continue;
}
RedisModule_CloseKey(real_key);
}

if (start_index) {
m_zslDeleteRangeByRank(tair_hash_obj->expire_index, 1, start_index);
if (!delEmptyTairHashIfNeeded(ctx, real_key, key, tair_hash_obj)) {
RedisModule_CloseKey(real_key);
}
delEmptyTairHashIfNeeded(ctx, NULL, key, tair_hash_obj);
}
m_listDelNode(keys, node);
}
Expand Down
12 changes: 10 additions & 2 deletions src/tairhash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ void _moduleAssert(const char *estr, const char *file, int line) {
*((char *)-1) = 'x';
}

inline struct TairHashVal *createTairHashVal(void) {
struct TairHashVal *createTairHashVal(void) {
struct TairHashVal *o;
o = RedisModule_Calloc(1, sizeof(*o));
return o;
}

inline void tairHashValRelease(struct TairHashVal *o) {
void tairHashValRelease(struct TairHashVal *o) {
if (o) {
if (o->value) {
RedisModule_FreeString(NULL, o->value);
Expand All @@ -85,6 +85,14 @@ int delEmptyTairHashIfNeeded(RedisModuleCtx *ctx, RedisModuleKey *key, RedisModu
return 0;
}

if (key == NULL) {
key = RedisModule_OpenKey(ctx, raw_key, REDISMODULE_WRITE);
if (RedisModule_KeyType(key) == REDISMODULE_KEYTYPE_EMPTY) {
RedisModule_CloseKey(key);
return 0;
}
}

if (redis_major_ver < 6 || (redis_major_ver == 6 && redis_minor_ver < 2)) {
/* See bugfix: https://github.com/redis/redis/pull/8617
https://github.com/redis/redis/pull/8097
Expand Down
Loading