From 66e62bb010281357f6c58dc2431697fd53fa6943 Mon Sep 17 00:00:00 2001 From: dapan1121 Date: Fri, 6 Jan 2023 19:19:31 +0800 Subject: [PATCH 1/7] fix: tsdb read invalid memory read issue --- source/dnode/vnode/src/tsdb/tsdbUtil.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/source/dnode/vnode/src/tsdb/tsdbUtil.c b/source/dnode/vnode/src/tsdb/tsdbUtil.c index 55703002b8..f30308845b 100644 --- a/source/dnode/vnode/src/tsdb/tsdbUtil.c +++ b/source/dnode/vnode/src/tsdb/tsdbUtil.c @@ -775,7 +775,16 @@ _exit: return code; } -void tRowMergerClear(SRowMerger *pMerger) { taosArrayDestroy(pMerger->pArray); } +void tRowMergerClear(SRowMerger *pMerger) { + for (int32_t iCol = 1; iCol < pMerger->pTSchema->numOfCols; iCol++) { + SColVal *pTColVal = taosArrayGet(pMerger->pArray, iCol); + if (IS_VAR_DATA_TYPE(pTColVal->type)) { + tFree(pTColVal->value.pData); + } + } + + taosArrayDestroy(pMerger->pArray); +} int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { int32_t code = 0; @@ -789,7 +798,17 @@ int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { if (key.version > pMerger->version) { if (!COL_VAL_IS_NONE(pColVal)) { - taosArraySet(pMerger->pArray, iCol, pColVal); + if (IS_VAR_DATA_TYPE(pColVal->type)) { + SColVal *pTColVal = taosArrayGet(pMerger->pArray, iCol); + + code = tRealloc(pTColVal->value.pData, pColVal->value.nData); + if (code) goto _exit; + + pTColVal->value.nData = pColVal->value.nData; + memcpy(pTColVal->value.pData, pColVal->value.pData, pTColVal.value.nData); + } else { + taosArraySet(pMerger->pArray, iCol, pColVal); + } } } else if (key.version < pMerger->version) { SColVal *tColVal = (SColVal *)taosArrayGet(pMerger->pArray, iCol); From d70e32e7d36b14a5f7497d33ef310a68580bdef9 Mon Sep 17 00:00:00 2001 From: dapan1121 Date: Sat, 7 Jan 2023 14:56:43 +0800 Subject: [PATCH 2/7] fix: compile issue --- source/dnode/vnode/src/tsdb/tsdbUtil.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/dnode/vnode/src/tsdb/tsdbUtil.c b/source/dnode/vnode/src/tsdb/tsdbUtil.c index f30308845b..112fbb61c6 100644 --- a/source/dnode/vnode/src/tsdb/tsdbUtil.c +++ b/source/dnode/vnode/src/tsdb/tsdbUtil.c @@ -801,11 +801,11 @@ int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { if (IS_VAR_DATA_TYPE(pColVal->type)) { SColVal *pTColVal = taosArrayGet(pMerger->pArray, iCol); - code = tRealloc(pTColVal->value.pData, pColVal->value.nData); + code = tRealloc(&pTColVal->value.pData, pColVal->value.nData); if (code) goto _exit; pTColVal->value.nData = pColVal->value.nData; - memcpy(pTColVal->value.pData, pColVal->value.pData, pTColVal.value.nData); + memcpy(pTColVal->value.pData, pColVal->value.pData, pTColVal->value.nData); } else { taosArraySet(pMerger->pArray, iCol, pColVal); } From 80586ad997994924f77810c79850d39d14171d41 Mon Sep 17 00:00:00 2001 From: dapan1121 Date: Mon, 9 Jan 2023 11:33:26 +0800 Subject: [PATCH 3/7] fix: invalid free issue --- source/dnode/vnode/src/inc/tsdb.h | 1 + source/dnode/vnode/src/tsdb/tsdbUtil.c | 12 ++++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/source/dnode/vnode/src/inc/tsdb.h b/source/dnode/vnode/src/inc/tsdb.h index 5a2e462c8c..77a3bb7a2f 100644 --- a/source/dnode/vnode/src/inc/tsdb.h +++ b/source/dnode/vnode/src/inc/tsdb.h @@ -573,6 +573,7 @@ struct STSDBRowIter { struct SRowMerger { STSchema *pTSchema; int64_t version; + bool merged; SArray *pArray; // SArray }; diff --git a/source/dnode/vnode/src/tsdb/tsdbUtil.c b/source/dnode/vnode/src/tsdb/tsdbUtil.c index 112fbb61c6..a9c31c19cb 100644 --- a/source/dnode/vnode/src/tsdb/tsdbUtil.c +++ b/source/dnode/vnode/src/tsdb/tsdbUtil.c @@ -776,10 +776,12 @@ _exit: } void tRowMergerClear(SRowMerger *pMerger) { - for (int32_t iCol = 1; iCol < pMerger->pTSchema->numOfCols; iCol++) { - SColVal *pTColVal = taosArrayGet(pMerger->pArray, iCol); - if (IS_VAR_DATA_TYPE(pTColVal->type)) { - tFree(pTColVal->value.pData); + if (pMerger->merged) { + for (int32_t iCol = 1; iCol < pMerger->pTSchema->numOfCols; iCol++) { + SColVal *pTColVal = taosArrayGet(pMerger->pArray, iCol); + if (IS_VAR_DATA_TYPE(pTColVal->type)) { + tFree(pTColVal->value.pData); + } } } @@ -801,6 +803,7 @@ int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { if (IS_VAR_DATA_TYPE(pColVal->type)) { SColVal *pTColVal = taosArrayGet(pMerger->pArray, iCol); + pTColVal->value.pData = NULL; code = tRealloc(&pTColVal->value.pData, pColVal->value.nData); if (code) goto _exit; @@ -821,6 +824,7 @@ int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { } pMerger->version = key.version; + pMerger->merged = true; _exit: return code; From df2175087b9c04c073d7b258b6fec037002a1676 Mon Sep 17 00:00:00 2001 From: dapan1121 Date: Mon, 9 Jan 2023 19:47:17 +0800 Subject: [PATCH 4/7] fix: memory leak and invalid read issue --- source/dnode/vnode/src/tsdb/tsdbUtil.c | 83 +++++++++++++++++++++----- 1 file changed, 67 insertions(+), 16 deletions(-) diff --git a/source/dnode/vnode/src/tsdb/tsdbUtil.c b/source/dnode/vnode/src/tsdb/tsdbUtil.c index a9c31c19cb..9bc903a0ba 100644 --- a/source/dnode/vnode/src/tsdb/tsdbUtil.c +++ b/source/dnode/vnode/src/tsdb/tsdbUtil.c @@ -682,6 +682,16 @@ int32_t tRowMergerInit2(SRowMerger *pMerger, STSchema *pResTSchema, TSDBROW *pRo } tsdbRowGetColVal(pRow, pTSchema, jCol++, pColVal); + if ((!COL_VAL_IS_NONE(pColVal)) && IS_VAR_DATA_TYPE(pColVal->type)) { + uint8_t *pVal = pColVal->value.pData; + + pColVal->value.pData = NULL; + code = tRealloc(&pColVal->value.pData, pColVal->value.nData); + if (code) goto _exit; + + memcpy(pColVal->value.pData, pVal, pColVal->value.nData); + } + if (taosArrayPush(pMerger->pArray, pColVal) == NULL) { code = TSDB_CODE_OUT_OF_MEMORY; goto _exit; @@ -720,12 +730,28 @@ int32_t tRowMergerAdd(SRowMerger *pMerger, TSDBROW *pRow, STSchema *pTSchema) { if (key.version > pMerger->version) { if (!COL_VAL_IS_NONE(pColVal)) { - taosArraySet(pMerger->pArray, iCol, pColVal); + if (IS_VAR_DATA_TYPE(pColVal->type)) { + SColVal *tColVal = taosArrayGet(pMerger->pArray, iCol); + code = tRealloc(&tColVal->value.pData, pColVal->value.nData); + if (code) return code; + + memcpy(tColVal->value.pData, pColVal->value.pData, pColVal->value.nData); + } else { + taosArraySet(pMerger->pArray, iCol, pColVal); + } } } else if (key.version < pMerger->version) { SColVal *tColVal = (SColVal *)taosArrayGet(pMerger->pArray, iCol); if (COL_VAL_IS_NONE(tColVal) && !COL_VAL_IS_NONE(pColVal)) { - taosArraySet(pMerger->pArray, iCol, pColVal); + if (IS_VAR_DATA_TYPE(pColVal->type)) { + SColVal *tColVal = taosArrayGet(pMerger->pArray, iCol); + code = tRealloc(&tColVal->value.pData, pColVal->value.nData); + if (code) return code; + + memcpy(tColVal->value.pData, pColVal->value.pData, pColVal->value.nData); + } else { + taosArraySet(pMerger->pArray, iCol, pColVal); + } } } else { ASSERT(0 && "dup versions not allowed"); @@ -765,6 +791,16 @@ int32_t tRowMergerInit(SRowMerger *pMerger, TSDBROW *pRow, STSchema *pTSchema) { // other for (int16_t iCol = 1; iCol < pTSchema->numOfCols; iCol++) { tsdbRowGetColVal(pRow, pTSchema, iCol, pColVal); + if ((!COL_VAL_IS_NONE(pColVal)) && IS_VAR_DATA_TYPE(pColVal->type)) { + uint8_t *pVal = pColVal->value.pData; + + pColVal->value.pData = NULL; + code = tRealloc(&pColVal->value.pData, pColVal->value.nData); + if (code) goto _exit; + + memcpy(pColVal->value.pData, pVal, pColVal->value.nData); + } + if (taosArrayPush(pMerger->pArray, pColVal) == NULL) { code = TSDB_CODE_OUT_OF_MEMORY; goto _exit; @@ -776,12 +812,10 @@ _exit: } void tRowMergerClear(SRowMerger *pMerger) { - if (pMerger->merged) { - for (int32_t iCol = 1; iCol < pMerger->pTSchema->numOfCols; iCol++) { - SColVal *pTColVal = taosArrayGet(pMerger->pArray, iCol); - if (IS_VAR_DATA_TYPE(pTColVal->type)) { - tFree(pTColVal->value.pData); - } + for (int32_t iCol = 1; iCol < pMerger->pTSchema->numOfCols; iCol++) { + SColVal *pTColVal = taosArrayGet(pMerger->pArray, iCol); + if (IS_VAR_DATA_TYPE(pTColVal->type)) { + tFree(pTColVal->value.pData); } } @@ -802,13 +836,17 @@ int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { if (!COL_VAL_IS_NONE(pColVal)) { if (IS_VAR_DATA_TYPE(pColVal->type)) { SColVal *pTColVal = taosArrayGet(pMerger->pArray, iCol); + if (!COL_VAL_IS_NULL(pColVal)) { + code = tRealloc(&pTColVal->value.pData, pColVal->value.nData); + if (code) goto _exit; - pTColVal->value.pData = NULL; - code = tRealloc(&pTColVal->value.pData, pColVal->value.nData); - if (code) goto _exit; - - pTColVal->value.nData = pColVal->value.nData; - memcpy(pTColVal->value.pData, pColVal->value.pData, pTColVal->value.nData); + pTColVal->value.nData = pColVal->value.nData; + memcpy(pTColVal->value.pData, pColVal->value.pData, pTColVal->value.nData); + } else { + tFree(pTColVal->value.pData); + pTColVal->value.pData = NULL; + taosArraySet(pMerger->pArray, iCol, pColVal); + } } else { taosArraySet(pMerger->pArray, iCol, pColVal); } @@ -816,7 +854,21 @@ int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { } else if (key.version < pMerger->version) { SColVal *tColVal = (SColVal *)taosArrayGet(pMerger->pArray, iCol); if (COL_VAL_IS_NONE(tColVal) && !COL_VAL_IS_NONE(pColVal)) { - taosArraySet(pMerger->pArray, iCol, pColVal); + if (IS_VAR_DATA_TYPE(pColVal->type)) { + if (!COL_VAL_IS_NULL(pColVal)) { + code = tRealloc(&tColVal->value.pData, pColVal->value.nData); + if (code) goto _exit; + + tColVal->value.nData = pColVal->value.nData; + memcpy(tColVal->value.pData, pColVal->value.pData, tColVal->value.nData); + } else { + tFree(tColVal->value.pData); + tColVal->value.pData = NULL; + taosArraySet(pMerger->pArray, iCol, pColVal); + } + } else { + taosArraySet(pMerger->pArray, iCol, pColVal); + } } } else { ASSERT(0); @@ -824,7 +876,6 @@ int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { } pMerger->version = key.version; - pMerger->merged = true; _exit: return code; From bfc483aa30ef156fba14eb4c8d8593371249d534 Mon Sep 17 00:00:00 2001 From: dapan1121 Date: Tue, 10 Jan 2023 11:32:13 +0800 Subject: [PATCH 5/7] fix: row merge flag issue --- source/dnode/vnode/src/tsdb/tsdbUtil.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/source/dnode/vnode/src/tsdb/tsdbUtil.c b/source/dnode/vnode/src/tsdb/tsdbUtil.c index 9bc903a0ba..ede1e9a424 100644 --- a/source/dnode/vnode/src/tsdb/tsdbUtil.c +++ b/source/dnode/vnode/src/tsdb/tsdbUtil.c @@ -682,13 +682,13 @@ int32_t tRowMergerInit2(SRowMerger *pMerger, STSchema *pResTSchema, TSDBROW *pRo } tsdbRowGetColVal(pRow, pTSchema, jCol++, pColVal); - if ((!COL_VAL_IS_NONE(pColVal)) && IS_VAR_DATA_TYPE(pColVal->type)) { + if ((!COL_VAL_IS_NONE(pColVal)) && (!COL_VAL_IS_NULL(pColVal)) && IS_VAR_DATA_TYPE(pColVal->type)) { uint8_t *pVal = pColVal->value.pData; pColVal->value.pData = NULL; code = tRealloc(&pColVal->value.pData, pColVal->value.nData); if (code) goto _exit; - + memcpy(pColVal->value.pData, pVal, pColVal->value.nData); } @@ -730,12 +730,14 @@ int32_t tRowMergerAdd(SRowMerger *pMerger, TSDBROW *pRow, STSchema *pTSchema) { if (key.version > pMerger->version) { if (!COL_VAL_IS_NONE(pColVal)) { - if (IS_VAR_DATA_TYPE(pColVal->type)) { + if ((!COL_VAL_IS_NULL(pColVal)) && IS_VAR_DATA_TYPE(pColVal->type)) { SColVal *tColVal = taosArrayGet(pMerger->pArray, iCol); code = tRealloc(&tColVal->value.pData, pColVal->value.nData); if (code) return code; - + + tColVal->value.nData = pColVal->value.nData; memcpy(tColVal->value.pData, pColVal->value.pData, pColVal->value.nData); + tColVal->flag = 0; } else { taosArraySet(pMerger->pArray, iCol, pColVal); } @@ -743,12 +745,13 @@ int32_t tRowMergerAdd(SRowMerger *pMerger, TSDBROW *pRow, STSchema *pTSchema) { } else if (key.version < pMerger->version) { SColVal *tColVal = (SColVal *)taosArrayGet(pMerger->pArray, iCol); if (COL_VAL_IS_NONE(tColVal) && !COL_VAL_IS_NONE(pColVal)) { - if (IS_VAR_DATA_TYPE(pColVal->type)) { - SColVal *tColVal = taosArrayGet(pMerger->pArray, iCol); + if ((!COL_VAL_IS_NULL(pColVal)) && IS_VAR_DATA_TYPE(pColVal->type)) { code = tRealloc(&tColVal->value.pData, pColVal->value.nData); if (code) return code; - + + tColVal->value.nData = pColVal->value.nData; memcpy(tColVal->value.pData, pColVal->value.pData, pColVal->value.nData); + tColVal->flag = 0; } else { taosArraySet(pMerger->pArray, iCol, pColVal); } @@ -791,7 +794,7 @@ int32_t tRowMergerInit(SRowMerger *pMerger, TSDBROW *pRow, STSchema *pTSchema) { // other for (int16_t iCol = 1; iCol < pTSchema->numOfCols; iCol++) { tsdbRowGetColVal(pRow, pTSchema, iCol, pColVal); - if ((!COL_VAL_IS_NONE(pColVal)) && IS_VAR_DATA_TYPE(pColVal->type)) { + if ((!COL_VAL_IS_NONE(pColVal)) && (!COL_VAL_IS_NULL(pColVal)) && IS_VAR_DATA_TYPE(pColVal->type)) { uint8_t *pVal = pColVal->value.pData; pColVal->value.pData = NULL; @@ -842,6 +845,7 @@ int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { pTColVal->value.nData = pColVal->value.nData; memcpy(pTColVal->value.pData, pColVal->value.pData, pTColVal->value.nData); + pTColVal->flag = 0; } else { tFree(pTColVal->value.pData); pTColVal->value.pData = NULL; @@ -861,6 +865,7 @@ int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { tColVal->value.nData = pColVal->value.nData; memcpy(tColVal->value.pData, pColVal->value.pData, tColVal->value.nData); + tColVal->flag = 0; } else { tFree(tColVal->value.pData); tColVal->value.pData = NULL; From 459d2932b1727fc3eee0cb529ae25ab2a01914f5 Mon Sep 17 00:00:00 2001 From: dapan1121 Date: Tue, 10 Jan 2023 13:44:24 +0800 Subject: [PATCH 6/7] fix: memcpy empty value issue --- source/dnode/vnode/src/tsdb/tsdbUtil.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/source/dnode/vnode/src/tsdb/tsdbUtil.c b/source/dnode/vnode/src/tsdb/tsdbUtil.c index ede1e9a424..86adc1dc80 100644 --- a/source/dnode/vnode/src/tsdb/tsdbUtil.c +++ b/source/dnode/vnode/src/tsdb/tsdbUtil.c @@ -689,7 +689,9 @@ int32_t tRowMergerInit2(SRowMerger *pMerger, STSchema *pResTSchema, TSDBROW *pRo code = tRealloc(&pColVal->value.pData, pColVal->value.nData); if (code) goto _exit; - memcpy(pColVal->value.pData, pVal, pColVal->value.nData); + if (pColVal->value.nData) { + memcpy(pColVal->value.pData, pVal, pColVal->value.nData); + } } if (taosArrayPush(pMerger->pArray, pColVal) == NULL) { @@ -736,7 +738,9 @@ int32_t tRowMergerAdd(SRowMerger *pMerger, TSDBROW *pRow, STSchema *pTSchema) { if (code) return code; tColVal->value.nData = pColVal->value.nData; - memcpy(tColVal->value.pData, pColVal->value.pData, pColVal->value.nData); + if (pColVal->value.nData) { + memcpy(tColVal->value.pData, pColVal->value.pData, pColVal->value.nData); + } tColVal->flag = 0; } else { taosArraySet(pMerger->pArray, iCol, pColVal); @@ -750,7 +754,9 @@ int32_t tRowMergerAdd(SRowMerger *pMerger, TSDBROW *pRow, STSchema *pTSchema) { if (code) return code; tColVal->value.nData = pColVal->value.nData; - memcpy(tColVal->value.pData, pColVal->value.pData, pColVal->value.nData); + if (pColVal->value.nData) { + memcpy(tColVal->value.pData, pColVal->value.pData, pColVal->value.nData); + } tColVal->flag = 0; } else { taosArraySet(pMerger->pArray, iCol, pColVal); @@ -800,8 +806,10 @@ int32_t tRowMergerInit(SRowMerger *pMerger, TSDBROW *pRow, STSchema *pTSchema) { pColVal->value.pData = NULL; code = tRealloc(&pColVal->value.pData, pColVal->value.nData); if (code) goto _exit; - - memcpy(pColVal->value.pData, pVal, pColVal->value.nData); + + if (pColVal->value.nData) { + memcpy(pColVal->value.pData, pVal, pColVal->value.nData); + } } if (taosArrayPush(pMerger->pArray, pColVal) == NULL) { @@ -844,7 +852,9 @@ int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { if (code) goto _exit; pTColVal->value.nData = pColVal->value.nData; - memcpy(pTColVal->value.pData, pColVal->value.pData, pTColVal->value.nData); + if (pTColVal->value.nData) { + memcpy(pTColVal->value.pData, pColVal->value.pData, pTColVal->value.nData); + } pTColVal->flag = 0; } else { tFree(pTColVal->value.pData); @@ -864,7 +874,9 @@ int32_t tRowMerge(SRowMerger *pMerger, TSDBROW *pRow) { if (code) goto _exit; tColVal->value.nData = pColVal->value.nData; - memcpy(tColVal->value.pData, pColVal->value.pData, tColVal->value.nData); + if (tColVal->value.nData) { + memcpy(tColVal->value.pData, pColVal->value.pData, tColVal->value.nData); + } tColVal->flag = 0; } else { tFree(tColVal->value.pData); From fd0d4bb83031d99807974616d413c367b0317625 Mon Sep 17 00:00:00 2001 From: dapan1121 <72057773+dapan1121@users.noreply.github.com> Date: Tue, 10 Jan 2023 15:39:16 +0800 Subject: [PATCH 7/7] Update tsdb.h fix: remove tmp field --- source/dnode/vnode/src/inc/tsdb.h | 1 - 1 file changed, 1 deletion(-) diff --git a/source/dnode/vnode/src/inc/tsdb.h b/source/dnode/vnode/src/inc/tsdb.h index 77a3bb7a2f..5a2e462c8c 100644 --- a/source/dnode/vnode/src/inc/tsdb.h +++ b/source/dnode/vnode/src/inc/tsdb.h @@ -573,7 +573,6 @@ struct STSDBRowIter { struct SRowMerger { STSchema *pTSchema; int64_t version; - bool merged; SArray *pArray; // SArray };