1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
|
From ce48e5a8da49d25f5490cc1be6a54f8d9db5f520 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20=C4=8Cun=C3=A1t?= <vladimir.cunat@nic.cz>
Date: Tue, 6 Apr 2021 17:28:52 +0200
Subject: [PATCH] treewide: fix unaligned access
Some less common HW (not x86, usually ARM) doesn't tolerate unaligned
access to memory and it's breakage of C as well.
It's easiest to check by meson's -Db_sanitize=undefined (on any HW).
I pushed millions of real-life QNAME+QTYPE queries over UDP in default
mode and the sanitizer seems clear now.
---
lib/cache/impl.h | 16 +++++++++++-----
lib/cache/nsec1.c | 14 ++++++++++----
lib/layer/iterate.c | 4 +++-
lib/selection.c | 4 ++--
4 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/lib/cache/impl.h b/lib/cache/impl.h
index b884c361..fca8653c 100644
--- a/lib/cache/impl.h
+++ b/lib/cache/impl.h
@@ -45,7 +45,10 @@ struct entry_h {
bool has_optout : 1; /**< Only for packets; persisted DNSSEC_OPTOUT. */
uint8_t _pad; /**< We need even alignment for data now. */
uint8_t data[];
-};
+/* Well, we don't really need packing or alignment changes,
+ * but due to LMDB the whole structure may not be stored at an aligned address,
+ * and we need compilers (for non-x86) to know it to avoid SIGBUS (test: UBSAN). */
+} __attribute__ ((packed,aligned(1)));
struct entry_apex;
/** Check basic consistency of entry_h for 'E' entries, not looking into ->data.
@@ -303,10 +306,13 @@ static inline int rdataset_dematerialized_size(const uint8_t *data, uint16_t *rd
assert(sizeof(count) == KR_CACHE_RR_COUNT_SIZE);
memcpy(&count, data, sizeof(count));
const uint8_t *rdata = data + sizeof(count);
- if (rdataset_count)
- *rdataset_count = count;
- for (int i = 0; i < count; ++i)
- rdata += knot_rdata_size(((knot_rdata_t *)rdata)->len);
+ if (rdataset_count) // memcpy is safe for unaligned case (on non-x86)
+ memcpy(rdataset_count, &count, sizeof(count));
+ for (int i = 0; i < count; ++i) {
+ __typeof__(((knot_rdata_t *)NULL)->len) len; // memcpy as above
+ memcpy(&len, rdata + offsetof(knot_rdata_t, len), sizeof(len));
+ rdata += knot_rdata_size(len);
+ }
return rdata - (data + sizeof(count));
}
diff --git a/lib/cache/nsec1.c b/lib/cache/nsec1.c
index 3985ca3b..ed242ca8 100644
--- a/lib/cache/nsec1.c
+++ b/lib/cache/nsec1.c
@@ -197,8 +197,14 @@ static const char * find_leq_NSEC1(struct kr_cache *cache, const struct kr_query
/* We know it starts before sname, so let's check the other end.
* 1. construct the key for the next name - kwz_hi. */
/* it's *full* name ATM */
- const knot_rdata_t *next = (const knot_rdata_t *)
- (eh->data + KR_CACHE_RR_COUNT_SIZE);
+ /* Technical complication: memcpy is safe for unaligned case (on non-x86) */
+ __typeof__(((knot_rdata_t *)NULL)->len) next_len;
+ const uint8_t *next_data;
+ { /* next points to knot_rdata_t but possibly unaligned */
+ const uint8_t *next = eh->data + KR_CACHE_RR_COUNT_SIZE;
+ memcpy(&next_len, next + offsetof(knot_rdata_t, len), sizeof(next_len));
+ next_data = next + offsetof(knot_rdata_t, data);
+ }
if (KR_CACHE_RR_COUNT_SIZE != 2 || get_uint16(eh->data) == 0) {
assert(false);
return "ERROR";
@@ -220,8 +226,8 @@ static const char * find_leq_NSEC1(struct kr_cache *cache, const struct kr_query
/* Lower-case chs; see also RFC 6840 5.1.
* LATER(optim.): we do lots of copying etc. */
knot_dname_t lower_buf[KNOT_DNAME_MAXLEN];
- ret = knot_dname_to_wire(lower_buf, next->data,
- MIN(next->len, KNOT_DNAME_MAXLEN));
+ ret = knot_dname_to_wire(lower_buf, next_data,
+ MIN(next_len, KNOT_DNAME_MAXLEN));
if (ret < 0) { /* _ESPACE */
return "range search found record with incorrect contents";
}
diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index 94342cfb..8fa8010e 100644
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -132,7 +132,9 @@ static bool is_valid_addr(const uint8_t *addr, size_t len)
{
if (len == sizeof(struct in_addr)) {
/* Filter ANY and 127.0.0.0/8 */
- uint32_t ip_host = ntohl(*(const uint32_t *)(addr));
+ uint32_t ip_host; /* Memcpy is safe for unaligned case (on non-x86) */
+ memcpy(&ip_host, addr, sizeof(ip_host));
+ ip_host = ntohl(ip_host);
if (ip_host == 0 || (ip_host & 0xff000000) == 0x7f000000) {
return false;
}
diff --git a/lib/selection.c b/lib/selection.c
index 56530cba..4b516bb8 100644
--- a/lib/selection.c
+++ b/lib/selection.c
@@ -155,8 +155,8 @@ struct rtt_state get_rtt_state(const uint8_t *ip, size_t len,
} else if (value.len != sizeof(struct rtt_state)) {
assert(false); // shouldn't happen but let's be more robust
state = default_rtt_state;
- } else {
- state = *(struct rtt_state *)value.data;
+ } else { // memcpy is safe for unaligned case (on non-x86)
+ memcpy(&state, value.data, sizeof(state));
}
free(key.data);
--
2.31.0
|
