Skip to content

RFC: Fix unaligned memory access #10

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

Open
vcgomes wants to merge 3 commits into
base: master
Choose a base branch
from
Open

RFC: Fix unaligned memory access #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9d7b15d
Allow CFLAGS to be overwritten by the user
vcgomes Apr 1, 2023
3e0c45f
Add safe unaligned memory access routines
vcgomes Apr 1, 2023
65e4b26
Replace raw memory access by their safer versions
vcgomes Apr 1, 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
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
NAME = ntpperf

CPPFLAGS = -D_GNU_SOURCE
CFLAGS = -O2 -Wall -g
CFLAGS ?= -O2 -Wall -g
LDFLAGS = -lpcap -lm

ifdef NTPPERF_NTS
Expand Down
38 changes: 19 additions & 19 deletions perf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,23 +226,23 @@ static bool process_response(struct pcap_pkthdr *header, const u_char *data, str
if (memcmp(config->dst_mac, data + 6, 6))
return false;

if (ntohs(*(uint16_t *)(data + 12)) != 0x0800)
if (ntohs(get_u16(data + 12)) != 0x0800)
return false;

data += 14;
if (data[0] >> 4 != 4 || (data[0] & 0xf) != 5 || data[9] != 17 ||
ntohl(*(uint32_t *)(data + 12)) != config->dst_address)
ntohl(get_u32(data + 12)) != config->dst_address)
return false;

dst_address = ntohl(*(uint32_t *)(data + 16));
src_port = ntohs(*(uint16_t *)(data + 20));
dst_port = ntohs(*(uint16_t *)(data + 22));
dst_address = ntohl(get_u32(data + 16));
src_port = ntohs(get_u16(data + 20));
dst_port = ntohs(get_u16(data + 22));
data += 28;

if (config->ptp_mcast) {
if (dst_address != PTP_MCAST_ADDR || (data[0] & 0xf) != 9)
return false;
dst_address = ntohl(*(uint32_t *)(data + 44));
dst_address = ntohl(get_u32(data + 44));
}

if ((dst_address ^ config->src_network) >> (32 - config->src_bits))
Expand All @@ -259,14 +259,14 @@ static bool process_response(struct pcap_pkthdr *header, const u_char *data, str
return false;

valid = header->caplen >= 90 && (data[0] & 0x7) == 0x4 &&
(*(uint64_t *)(data + 24) & -2ULL) == (client->local_id & -2ULL) &&
(get_u64(data + 24) & -2ULL) == (client->local_id & -2ULL) &&
(!config->nts.cookie || header->len > 90 + 4 + 32 + 4 + 16 + 16 + 4);
if (valid) {
if (config->mode == NTP_INTERLEAVED)
client->remote_id = *(uint64_t *)(data + 32);
client->remote_id = get_u64(data + 32);
else
client->remote_id = *(uint64_t *)(data + 40);
client->remote_rx = convert_ntp_ts(*(uint64_t *)(data + 32));
client->remote_id = get_u64(data + 40);
client->remote_rx = convert_ntp_ts(get_u64(data + 32));
client->local_rx = local_rx;
}
break;
Expand All @@ -277,7 +277,7 @@ static bool process_response(struct pcap_pkthdr *header, const u_char *data, str

ptp_type = data[0] & 0xf;
valid = header->caplen >= 86 && data[1] == 2 &&
*(uint16_t *)(data + 30) == (uint16_t)client->local_id &&
get_u16(data + 30) == (uint16_t)client->local_id &&
((ptp_type == 9 && dst_port == 320) ||
(config->mode == PTP_NSM &&
((ptp_type == 0 && dst_port == 319) ||
Expand All @@ -290,9 +290,9 @@ static bool process_response(struct pcap_pkthdr *header, const u_char *data, str
client->local_rx = local_rx;
break;
case 9:
client->remote_rx = convert_ptp_ts(*(uint16_t *)(data + 34),
*(uint32_t *)(data + 36),
*(uint32_t *)(data + 40));
client->remote_rx = convert_ptp_ts(get_u16(data + 34),
get_u32(data + 36),
get_u32(data + 40));
/* Fall through */
case 8:
memset(&client->local_rx, 0, sizeof client->local_rx);
Expand All @@ -317,7 +317,7 @@ static bool process_response(struct pcap_pkthdr *header, const u_char *data, str
switch (config->mode) {
case NTP_BASIC:
case NTP_INTERLEAVED:
if (*(uint64_t *)(data + 24) == client->local_id) {
if (get_u64(data + 24) == client->local_id) {
stats->basic_responses++;
if (config->mode != NTP_BASIC)
return true;
Expand All @@ -330,7 +330,7 @@ static bool process_response(struct pcap_pkthdr *header, const u_char *data, str
local_rx = prev_local_rx;
}

remote_tx = convert_ntp_ts(*(uint64_t *)(data + 40));
remote_tx = convert_ntp_ts(get_u64(data + 40));
break;
case PTP_DELAY:
case PTP_NSM:
Expand All @@ -346,9 +346,9 @@ static bool process_response(struct pcap_pkthdr *header, const u_char *data, str
stats->sync_responses++;

remote_rx = client->remote_rx;
remote_tx = convert_ptp_ts(*(uint16_t *)(data + 34),
*(uint32_t *)(data + 36),
*(uint32_t *)(data + 40));
remote_tx = convert_ptp_ts(get_u16(data + 34),
get_u32(data + 36),
get_u32(data + 40));
break;
case 9:
stats->delay_responses++;
Expand Down
67 changes: 34 additions & 33 deletions sender.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,27 +141,28 @@ static int make_packet(struct sender_request *request, struct sender_config *con
/* Ethernet header */
memcpy(buf + 0, config->dst_mac, 6);
memcpy(buf + 6, config->src_mac, 6);
*(uint16_t *)(buf + 12) = htons(0x0800);
put_u16(htons(0x0800), buf + 12);
buf += 14, len += 14;

/* IP header */
memcpy(buf, "\x45\x00\x00\x00\xd7\xe9\x40\x00\x40\x11", 10);
*(uint16_t *)(buf + 2) = htons(20 + 8 + data_len);
*(uint32_t *)(buf + 12) = htonl(request->src_address);
*(uint32_t *)(buf + 16) = htonl(config->dst_address);
put_u16(htons(20 + 8 + data_len), buf + 2);
put_u32(htonl(request->src_address), buf + 12);
put_u32(htonl(config->dst_address), buf + 16);

for (i = 0; i < 10; i++)
sum += ((uint16_t *)buf)[i];
for (i = 0; i < 20; i += 2)
sum += get_u16(buf + i);
while ((carry = sum >> 16))
sum = (sum & 0xffff) + carry;

*(uint16_t *)(buf + 10) = ~sum;
put_u16(~sum, buf + 10);

buf += 20, len += 20;

/* UDP header and data */
*(uint16_t *)(buf + 0) = htons(src_port);
*(uint16_t *)(buf + 2) = htons(dst_port);
*(uint16_t *)(buf + 4) = htons(8 + data_len);
put_u16(htons(src_port), buf + 0);
put_u16(htons(dst_port), buf + 2);
put_u16(htons(8 + data_len), buf + 4);
buf += 8, len += 8;

assert(max_len >= len + data_len);
Expand All @@ -170,21 +171,21 @@ static int make_packet(struct sender_request *request, struct sender_config *con
case NTP_BASIC:
case NTP_INTERLEAVED:
buf[0] = 0xe3;
*(uint64_t *)(buf + 24) = request->remote_id;
*(uint64_t *)(buf + 32) = request->local_id ^ 1;
*(uint64_t *)(buf + 40) = request->local_id;
put_u64(request->remote_id, buf + 24);
put_u64(request->local_id ^ 1, buf + 32);
put_u64(request->local_id, buf + 40);
auth = buf + 48;
break;
case PTP_NSM:
*(uint32_t *)(buf + 44) = htonl(0x21fe0000);
put_u32(htonl(0x21fe0000), buf + 44);
/* Fall through */
case PTP_DELAY:
*(uint16_t *)(buf + 0) = htons(0x0102);
*(uint16_t *)(buf + 2) = htons(data_len);
*(uint8_t *)(buf + 4) = config->ptp_domain;
put_u16(htons(0x0102), buf + 0);
put_u16(htons(data_len), buf + 2);
put_u8(config->ptp_domain, buf + 4);
buf[6] = config->ptp_mcast ? 0 : 0x4;
*(uint32_t *)(buf + 20) = htonl(request->src_address);
*(uint16_t *)(buf + 30) = request->local_id;
put_u32(htonl(request->src_address), buf + 20);
put_u16(request->local_id, buf + 30);
buf[32] = 0x1;
break;
default:
Expand All @@ -196,27 +197,27 @@ static int make_packet(struct sender_request *request, struct sender_config *con
size_t clen;

/* Unique Identifier */
*(uint16_t *)(auth + 0) = htons(0x0104);
*(uint16_t *)(auth + 2) = htons(4 + 32);
*(uint32_t *)(auth + 4) = random();
*(uint32_t *)(auth + 8) = random();
put_u16(htons(0x0104), auth + 0);
put_u16(htons(4 + 32), auth + 2);
put_u32(random(), auth + 4);
put_u32(random(), auth + 8);
auth += 4 + 32;

/* Cookie */
*(uint16_t *)(auth + 0) = htons(0x0204);
*(uint16_t *)(auth + 2) = htons(4 + nts->cookie_len);
put_u16(htons(0x0204), auth + 0);
put_u16(htons(4 + nts->cookie_len), auth + 2);
memcpy(auth + 4, nts->cookie, nts->cookie_len);
auth += 4 + nts->cookie_len;

/* Authenticator */
*(uint16_t *)(auth + 0) = htons(0x0404);
*(uint16_t *)(auth + 2) = htons(4 + 4 + 16 + 16);
*(uint16_t *)(auth + 4) = htons(16);
*(uint16_t *)(auth + 6) = htons(16);
*(uint32_t *)(auth + 8) = random();
*(uint32_t *)(auth + 12) = random();
*(uint32_t *)(auth + 16) = random();
*(uint32_t *)(auth + 20) = random();
put_u16(htons(0x0404), auth + 0);
put_u16(htons(4 + 4 + 16 + 16), auth + 2);
put_u16(htons(16), auth + 4);
put_u16(htons(16), auth + 6);
put_u32(random(), auth + 8);
put_u32(random(), auth + 12);
put_u32(random(), auth + 16);
put_u32(random(), auth + 20);
clen = 16;
if (gnutls_aead_cipher_encrypt(nts->cipher,
auth + 4 + 4, 16, buf, auth - buf, 0,
Expand Down
57 changes: 57 additions & 0 deletions sender.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,61 @@ int sender_start(struct sender_config *config);
bool sender_send_requests(int sender_fd, struct sender_request *requests, int num);
void sender_stop(int sender_fd);

#define GET_UNALIGNED(ptr) __extension__ \
({ \
struct __attribute__((packed)) { \
__typeof__(*(ptr)) __v; \
} *__p = (__typeof__(__p)) (ptr); \
__p->__v; \
})

#define PUT_UNALIGNED(val, ptr) \
do { \
struct __attribute__((packed)) { \
__typeof__(*(ptr)) __v; \
} *__p = (__typeof__(__p)) (ptr); \
__p->__v = (val); \
} while(0)


static inline uint8_t get_u8(const void *ptr)
{
return *((const uint8_t *) ptr);
}

static inline void put_u8(uint8_t val, void *ptr)
{
*((uint8_t *) ptr) = val;
}

static inline uint16_t get_u16(const void *ptr)
{
return GET_UNALIGNED((const uint16_t *) ptr);
}

static inline uint32_t get_u32(const void *ptr)
{
return GET_UNALIGNED((const uint32_t *) ptr);
}

static inline uint32_t get_u64(const void *ptr)
{
return GET_UNALIGNED((const uint32_t *) ptr);
}

static inline void put_u16(uint16_t val, void *ptr)
{
PUT_UNALIGNED(val, (uint16_t *) ptr);
}

static inline void put_u32(uint32_t val, void *ptr)
{
PUT_UNALIGNED(val, (uint32_t *) ptr);
}

static inline void put_u64(uint64_t val, void *ptr)
{
PUT_UNALIGNED(val, (uint64_t *) ptr);
}

#endif