fixeria has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-pcu/+/43143?usp=email )
Change subject: llc: fix misaligned MetaInfo access in msgb headroom
......................................................................
llc: fix misaligned MetaInfo access in msgb headroom
struct MetaInfo (two struct timespec) requires 8-byte alignment on
some ABIs (e.g. 32-bit ARM with a 64-bit time_t), but msgb->head is
only guaranteed byte alignment: struct msgb's own size before the
flexible _data[] member is not a multiple of 8, so storing MetaInfo
directly at msg->head can misalign it, triggering UBSan
-fsanitize=alignment reports wherever recv_time/expire_time
are subsequently dereferenced.
Reserve extra headroom for worst-case alignment padding and
store/retrieve MetaInfo via the next 8-byte-aligned address within
that headroom instead of assuming msg->head itself is suitably
aligned.
Change-Id: I39abce68c5678704eea2ad1e330419f645aa4cfd
Related: OS#6858
---
M src/llc.c
1 file changed, 34 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/43/43143/1
diff --git a/src/llc.c b/src/llc.c
index 478d23d..8107c0c 100644
--- a/src/llc.c
+++ b/src/llc.c
@@ -17,6 +17,7 @@
#include <stdio.h>
+#include <stdint.h>
#include <osmocom/core/msgb.h>
@@ -25,6 +26,31 @@
#include "pcu_utils.h"
#include "llc.h"
+/* struct msgb's flexible '_data' array is only guaranteed to start at an
+ * address suitable for 'unsigned char', not for arbitrary types: on some
+ * ABIs (e.g. 32-bit ARM with a 64-bit time_t) 'struct MetaInfo' requires
+ * stricter (8-byte) alignment than that, courtesy of the embedded
+ * 'struct timespec' members, so msgb->head itself cannot be assumed to be
+ * a valid storage location for it. Reserve extra headroom for worst-case
+ * padding and store/retrieve MetaInfo at the next aligned address instead
+ * of at msgb->head directly. */
+#define META_INFO_ALIGN _Alignof(struct MetaInfo)
+#define META_INFO_HEADROOM (sizeof(struct MetaInfo) + META_INFO_ALIGN - 1)
+
+/* Allocate a msgb with enough headroom to hold a MetaInfo (plus worst-case
+ * alignment padding) followed by 'len' bytes of LLC PDU data. */
+static inline struct msgb *llc_msgb_meta_info_alloc(uint16_t len, const char
*name)
+{
+ return msgb_alloc_headroom(META_INFO_HEADROOM + len,
META_INFO_HEADROOM, name);
+}
+
+static inline struct MetaInfo *llc_msgb_meta_info_get(const struct msgb *msg)
+{
+ uintptr_t p = (uintptr_t)msg->head;
+ p = (p + (META_INFO_ALIGN - 1)) & ~(uintptr_t)(META_INFO_ALIGN - 1);
+ return (struct MetaInfo *)p;
+}
+
void llc_init(struct gprs_llc *llc)
{
llc_reset(llc);
@@ -148,12 +174,12 @@
struct gprs_llc_hdr *llc_hdr;
enum gprs_llc_queue_prio prio;
- llc_msg = msgb_alloc_headroom(sizeof(*meta_storage) + len,
sizeof(*meta_storage), "llc_pdu_queue");
+ llc_msg = llc_msgb_meta_info_alloc(len, "llc_pdu_queue");
if (!llc_msg)
return -ENOMEM;
/* Fist set up MetaInfo in the msgb headroom: */
- meta_storage = (struct MetaInfo *)llc_msg->head;
+ meta_storage = llc_msgb_meta_info_get(llc_msg);
osmo_clock_gettime(CLOCK_MONOTONIC, &meta_storage->recv_time);
meta_storage->expire_time = *expire_time;
@@ -215,8 +241,8 @@
msg = msg1;
msg1 = NULL;
} else {
- const struct MetaInfo *mi1 = (struct MetaInfo
*)msg1->head;
- const struct MetaInfo *mi2 = (struct MetaInfo
*)msg2->head;
+ const struct MetaInfo *mi1 =
llc_msgb_meta_info_get(msg1);
+ const struct MetaInfo *mi2 =
llc_msgb_meta_info_get(msg2);
if (timespeccmp(&mi2->recv_time,
&mi1->recv_time, >)) {
msg = msg1;
@@ -250,10 +276,10 @@
struct MetaInfo *meta_storage;
unsigned int len = llc_frame_length(llc);
- llc_msg = msgb_alloc_headroom(sizeof(*meta_storage) + len,
sizeof(*meta_storage), "llc_pdu_queue");
+ llc_msg = llc_msgb_meta_info_alloc(len, "llc_pdu_queue");
OSMO_ASSERT(llc_msg);
- meta_storage = (struct MetaInfo *)llc_msg->head;
+ meta_storage = llc_msgb_meta_info_get(llc_msg);
memcpy(meta_storage, &llc->meta_info, sizeof(struct MetaInfo));
memcpy(msgb_put(llc_msg, len), llc->frame, len);
@@ -283,7 +309,7 @@
if (!msg)
return NULL;
- meta_storage = (struct MetaInfo *)msg->head;
+ meta_storage = llc_msgb_meta_info_get(msg);
q->queue_size -= 1;
q->queue_octets -= msgb_length(msg);
@@ -316,7 +342,7 @@
timespecadd(&tv_now, &hyst_delta, &tv_now2);
while ((msg = llc_queue_pick_msg(q, &prio))) {
- info = (const struct MetaInfo *)msg->head;
+ info = llc_msgb_meta_info_get(msg);
const struct timespec *tv_disc = &info->expire_time;
const struct timespec *tv_recv = &info->recv_time;
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/43143?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I39abce68c5678704eea2ad1e330419f645aa4cfd
Gerrit-Change-Number: 43143
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <[email protected]>
