Browse Source

Fix UB of unaligned loads/stores and signed shifts. (#16628)

* Ignore build/ dir.

This directory is the default dir for many LSPs and for IDES using
cmake. "Reserve" it by ignoring it in .gitignore.

* Fix format specifier.

* Use unsigned literals when shifting.

* Do not sanitize shifts in libjudy.

* Fix unaligned loads/stores of dbengine's CRCs

* Fix unaligned load when partitioning metrics.

* Use unsigned literals when shifting.
vkalintiris 1 year ago
parent
commit
9ff1b5bdae

+ 3 - 0
.gitignore

@@ -256,3 +256,6 @@ database/engine/journalfile_v2_virtmemb.ksy
 libnetdata/gorilla/gorilla_benchmark
 libnetdata/gorilla/gorilla_fuzzer
 libnetdata/gorilla/fuzz-*.log
+
+# ignore build/ directory (default dir for many IDEs/LSPs)
+build/

+ 1 - 4
collectors/xenstat.plugin/xenstat_plugin.c

@@ -1032,10 +1032,7 @@ int main(int argc, char **argv) {
         if(unlikely(netdata_exit)) break;
 
         if(unlikely(debug && iteration))
-            fprintf(stderr, "xenstat.plugin: iteration %zu, dt %llu usec\n"
-                    , iteration
-                    , dt
-            );
+            fprintf(stderr, "xenstat.plugin: iteration %zu, dt %lu usec\n", iteration, dt);
 
         if(likely(xhandle)) {
             if(unlikely(debug)) fprintf(stderr, "xenstat.plugin: calling xenstat_collect()\n");

+ 5 - 2
database/engine/metric.c

@@ -106,8 +106,11 @@ static inline void mrg_stats_size_judyhs_removed_uuid(MRG *mrg, size_t partition
 
 static inline size_t uuid_partition(MRG *mrg __maybe_unused, uuid_t *uuid) {
     uint8_t *u = (uint8_t *)uuid;
-    size_t *n = (size_t *)&u[UUID_SZ - sizeof(size_t)];
-    return *n % mrg->partitions;
+
+    size_t n;
+    memcpy(&n, &u[UUID_SZ - sizeof(size_t)], sizeof(size_t));
+
+    return n % mrg->partitions;
 }
 
 static inline time_t mrg_metric_get_first_time_s_smart(MRG *mrg __maybe_unused, METRIC *metric) {

+ 4 - 2
database/engine/rrdenginelib.h

@@ -67,12 +67,14 @@ static inline unsigned long ulong_compare_and_swap(volatile unsigned long *ptr,
 
 static inline int crc32cmp(void *crcp, uLong crc)
 {
-    return (*(uint32_t *)crcp != crc);
+    uint32_t loaded_crc;
+    memcpy(&loaded_crc, crcp, sizeof(loaded_crc));
+    return (loaded_crc != crc);
 }
 
 static inline void crc32set(void *crcp, uLong crc)
 {
-    *(uint32_t *)crcp = crc;
+    memcpy(crcp, &crc, sizeof(crc));
 }
 
 int check_file_properties(uv_file file, uint64_t *file_size, size_t min_size);

+ 1 - 0
libnetdata/libjudy/src/JudyL/JudyLCascade.c

@@ -311,6 +311,7 @@ static int j__udyStageJBBtoJBB(
 //
 // NOTE:  Caller must release the Leaf2 that was passed in.
 
+__attribute__((no_sanitize("shift")))
 FUNCTION static Pjlb_t j__udyJLL2toJLB1(
 	uint16_t * Pjll,	// array of 16-bit indexes.
 #ifdef JUDYL

+ 2 - 0
libnetdata/libjudy/src/JudyL/JudyLDecascade.c

@@ -345,6 +345,7 @@ FUNCTION int j__udyBranchUToBranchB(
 // allocation and free, in order to allow the caller to continue with a LeafB1
 // if allocation fails.
 
+__attribute__((no_sanitize("shift")))
 FUNCTION int j__udyLeafB1ToLeaf1(
 	Pjp_t	  Pjp,		// points to LeafB1 to shrink.
 	Pvoid_t	  Pjpm)		// for global accounting.
@@ -431,6 +432,7 @@ FUNCTION int j__udyLeafB1ToLeaf1(
 // TBD:  In this and all following functions, the caller should already be able
 // to compute the Pop1 return value, so why return it?
 
+__attribute__((no_sanitize("shift")))
 FUNCTION Word_t  j__udyLeaf1ToLeaf2(
 	uint16_t * PLeaf2,	// destination uint16_t * Index portion of leaf.
 #ifdef JUDYL

+ 1 - 0
libnetdata/libjudy/src/JudyL/JudyLDel.c

@@ -147,6 +147,7 @@ extern Word_t   j__udyLLeaf7ToLeafW(Pjlw_t,     Pjv_t, Pjp_t, Word_t, Pvoid_t);
 
 DBGCODE(uint8_t parentJPtype;)          // parent branch JP type.
 
+__attribute__((no_sanitize("shift")))
 FUNCTION static int j__udyDelWalk(
         Pjp_t   Pjp,            // current JP under which to delete.
         Word_t  Index,          // to delete.

+ 2 - 0
libnetdata/libjudy/src/JudyL/JudyLGet.c

@@ -44,6 +44,8 @@
 // See the manual entry for details.  Note support for "shortcut" entries to
 // trees known to start with a JPM.
 
+__attribute__((no_sanitize("shift")))
+
 #ifdef JUDY1
 
 #ifdef JUDYGETINLINE

+ 1 - 0
libnetdata/libjudy/src/JudyL/JudyLIns.c

@@ -152,6 +152,7 @@ extern int j__udyLInsertBranch(Pjp_t Pjp, Word_t Index, Word_t Btype, Pjpm_t);
 // Return -1 for error (details in JPM), 0 for Index already inserted, 1 for
 // new Index inserted.
 
+__attribute__((no_sanitize("shift")))
 FUNCTION static int j__udyInsWalk(
         Pjp_t   Pjp,            // current JP to descend.
         Word_t  Index,          // to insert.

+ 6 - 6
libnetdata/storage_number/storage_number.h

@@ -116,10 +116,10 @@ storage_number pack_storage_number(NETDATA_DOUBLE value, SN_FLAGS flags) __attri
 static inline NETDATA_DOUBLE unpack_storage_number(storage_number value) __attribute__((const));
 
 //                                                          sign       div/mul      <--- multiplier / divider --->     10/100       RESET      EXISTS     VALUE
-#define STORAGE_NUMBER_POSITIVE_MAX_RAW (storage_number)( (0 << 31) | (1 << 30) | (1 << 29) | (1 << 28) | (1 << 27) | (1 << 26) | (0 << 25) | (1 << 24) | 0x00ffffff )
-#define STORAGE_NUMBER_POSITIVE_MIN_RAW (storage_number)( (0 << 31) | (0 << 30) | (1 << 29) | (1 << 28) | (1 << 27) | (0 << 26) | (0 << 25) | (1 << 24) | 0x00000001 )
-#define STORAGE_NUMBER_NEGATIVE_MAX_RAW (storage_number)( (1 << 31) | (0 << 30) | (1 << 29) | (1 << 28) | (1 << 27) | (0 << 26) | (0 << 25) | (1 << 24) | 0x00000001 )
-#define STORAGE_NUMBER_NEGATIVE_MIN_RAW (storage_number)( (1 << 31) | (1 << 30) | (1 << 29) | (1 << 28) | (1 << 27) | (1 << 26) | (0 << 25) | (1 << 24) | 0x00ffffff )
+#define STORAGE_NUMBER_POSITIVE_MAX_RAW (storage_number)( (0U << 31) | (1U << 30) | (1U << 29) | (1U << 28) | (1U << 27) | (1U << 26) | (0U << 25) | (1U << 24) | 0x00ffffff )
+#define STORAGE_NUMBER_POSITIVE_MIN_RAW (storage_number)( (0U << 31) | (0U << 30) | (1U << 29) | (1U << 28) | (1U << 27) | (0U << 26) | (0U << 25) | (1U << 24) | 0x00000001 )
+#define STORAGE_NUMBER_NEGATIVE_MAX_RAW (storage_number)( (1U << 31) | (0U << 30) | (1U << 29) | (1U << 28) | (1U << 27) | (0U << 26) | (0U << 25) | (1U << 24) | 0x00000001 )
+#define STORAGE_NUMBER_NEGATIVE_MIN_RAW (storage_number)( (1U << 31) | (1U << 30) | (1U << 29) | (1U << 28) | (1U << 27) | (1U << 26) | (0U << 25) | (1U << 24) | 0x00ffffff )
 
 // accepted accuracy loss
 #define ACCURACY_LOSS_ACCEPTED_PERCENT 0.0001
@@ -155,10 +155,10 @@ static inline NETDATA_DOUBLE unpack_storage_number(storage_number value) {
     // bit 25 SN_FLAG_NOT_ANOMALOUS
 
     // bit 30, 29, 28 = (multiplier or divider) 0-7 (8 total)
-    int mul = (int)((value & ((1<<29)|(1<<28)|(1<<27))) >> 27);
+    int mul = (int)((value & ((1U<<29)|(1U<<28)|(1U<<27))) >> 27);
 
     // bit 24 to bit 1 = the value, so remove all other bits
-    value ^= value & ((1<<31)|(1<<30)|(1<<29)|(1<<28)|(1<<27)|(1<<26)|(1<<25)|(1<<24));
+    value ^= value & ((1U <<31)|(1U <<30)|(1U <<29)|(1U <<28)|(1U <<27)|(1U <<26)|(1U <<25)|(1U<<24));
 
     NETDATA_DOUBLE n = value;
 

Some files were not shown because too many files changed in this diff