BUG/MEDIUM: quic: don't blindly rely on unaligned accesses

There are several places where the QUIC low-level code performs unaligned
accesses by casting unaligned char* pointers to uint32_t, but this is
totally forbidden as it only works on machines that support unaligned
accesses, and either crashes on other ones (SPARC, MIPS), can result in
reading garbage (ARMv5) or be very slow due to the access being emulated
(RISC-V). We do have functions for this, such as read_u32() and write_u32()
that rely on the compiler's knowledge of the machine's capabilities to
either perform an unaligned access or do it one byte at a time.

This must be backported at least as far as 2.6. Some of the code moved a
few times since, so in order to figure the points that need to be fixed,
one may look for a forced pointer cast without having verified that either
the machine is compatible or that the pointer is aligned using this:

  $ git grep 'uint[36][24]_t \*)'

Or build and run the code on a MIPS or SPARC and perform requests using
curl to see if they work or crash with a bus error. All the places fixed
in this commit were found thanks to an immediate crash on the first
request.

This was tagged medium because the affected archs are not the most common
ones where QUIC will be found these days.

src/quic_retry.c

@@ -60,7 +60,7 @@ static int quic_generate_retry_token_aad(unsigned char *aad,
 	unsigned char *p;
 
 	p = aad;
-	*(uint32_t *)p = htonl(version);
+	write_u32(p, htonl(version));
 	p += sizeof version;
 	p += quic_saddr_cpy(p, addr);
 	memcpy(p, cid->data, cid->len);

src/quic_rx.c

@@ -1462,7 +1462,7 @@ static inline int quic_read_uint32(uint32_t *val,
 	if (end - *buf < sizeof *val)
 		return 0;
 
-	*val = ntohl(*(uint32_t *)*buf);
+	*val = ntohl(read_u32(*buf));
 	*buf += sizeof *val;
 
 	return 1;

src/quic_tp.c

@@ -171,23 +171,23 @@ static int quic_transport_param_dec_version_info(struct tp_version_information *
                                                  const unsigned char *end, int server)
 {
 	size_t tp_len = end - *buf;
-	const uint32_t *ver, *others;
+	const unsigned char *ver, *others;
 
 	/* <tp_len> must be a multiple of sizeof(uint32_t) */
 	if (tp_len < sizeof tp->chosen || (tp_len & 0x3))
 		return 0;
 
-	tp->chosen = ntohl(*(uint32_t *)*buf);
+	tp->chosen = ntohl(read_u32(*buf));
 	/* Must not be null */
 	if (!tp->chosen)
 		return 0;
 
 	*buf += sizeof tp->chosen;
-	others = (const uint32_t *)*buf;
+	others = *buf;
 
 	/* Others versions must not be null */
-	for (ver = others; ver < (const uint32_t *)end; ver++) {
-		if (!*ver)
+	for (ver = others; ver < end; ver += 4) {
+		if (!read_u32(ver))
 			return 0;
 	}
 
@@ -195,19 +195,19 @@ static int quic_transport_param_dec_version_info(struct tp_version_information *
 		/* TODO: not supported */
 		return 0;
 
-	for (ver = others; ver < (const uint32_t *)end; ver++) {
+	for (ver = others; ver < end; ver += 4) {
 		if (!tp->negotiated_version) {
 			int i;
 
 			for (i = 0; i < quic_versions_nb; i++) {
-				if (ntohl(*ver) == quic_versions[i].num) {
+				if (ntohl(read_u32(ver)) == quic_versions[i].num) {
 					tp->negotiated_version = &quic_versions[i];
 					break;
 				}
 			}
 		}
 
-		if (preferred_version && ntohl(*ver) == preferred_version->num) {
+		if (preferred_version && ntohl(read_u32(ver)) == preferred_version->num) {
 			tp->negotiated_version = preferred_version;
 			goto out;
 		}

src/quic_tx.c

@@ -1323,7 +1323,7 @@ static inline int quic_write_uint32(unsigned char **buf,
 	if (end - *buf < sizeof val)
 		return 0;
 
-	*(uint32_t *)*buf = htonl(val);
+	write_u32(*buf, htonl(val));
 	*buf += sizeof val;
 
 	return 1;