From 0e83982aeb97cf5a8d4b36c3e642504a0ace3586 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Tue, 6 Aug 2019 14:41:01 +0200 Subject: [PATCH] Correct ACKNACK, GAP, NACKFRAG size calculation Commit 3afce30c370532d27ffe1838b6618f0649bfbda5 introduced an error in the calculation of the size of these submessages, making them (and requiring them to be) larger than correct and putting the "count" field at the wrong offset, breaking interoperability. Signed-off-by: Erik Boasson --- src/core/ddsi/include/dds/ddsi/q_protocol.h | 6 +++--- src/core/ddsi/src/q_receive.c | 10 ++++------ src/core/ddsi/src/q_xevent.c | 7 +++---- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/core/ddsi/include/dds/ddsi/q_protocol.h b/src/core/ddsi/include/dds/ddsi/q_protocol.h index 9d5495c..5a5d6e0 100644 --- a/src/core/ddsi/include/dds/ddsi/q_protocol.h +++ b/src/core/ddsi/include/dds/ddsi/q_protocol.h @@ -232,7 +232,7 @@ typedef struct AckNack { /* nn_count_t count; */ } AckNack_t; #define ACKNACK_FLAG_FINAL 0x02u -#define ACKNACK_SIZE(numbits) (offsetof (AckNack_t, bits) + NN_SEQUENCE_NUMBER_SET_SIZE (numbits) + 4) +#define ACKNACK_SIZE(numbits) (offsetof (AckNack_t, bits) + NN_SEQUENCE_NUMBER_SET_BITS_SIZE (numbits) + 4) #define ACKNACK_SIZE_MAX ACKNACK_SIZE (256u) typedef struct Gap { @@ -243,7 +243,7 @@ typedef struct Gap { nn_sequence_number_set_header_t gapList; uint32_t bits[]; } Gap_t; -#define GAP_SIZE(numbits) (offsetof (Gap_t, bits) + NN_SEQUENCE_NUMBER_SET_SIZE (numbits)) +#define GAP_SIZE(numbits) (offsetof (Gap_t, bits) + NN_SEQUENCE_NUMBER_SET_BITS_SIZE (numbits)) #define GAP_SIZE_MAX GAP_SIZE (256u) typedef struct InfoTS { @@ -281,7 +281,7 @@ typedef struct NackFrag { uint32_t bits[]; /* nn_count_t count; */ } NackFrag_t; -#define NACKFRAG_SIZE(numbits) (offsetof (NackFrag_t, fragmentNumberState) + NN_FRAGMENT_NUMBER_SET_SIZE (numbits) + 4) +#define NACKFRAG_SIZE(numbits) (offsetof (NackFrag_t, bits) + NN_FRAGMENT_NUMBER_SET_BITS_SIZE (numbits) + 4) #define NACKFRAG_SIZE_MAX NACKFRAG_SIZE (256u) typedef struct PT_InfoContainer { diff --git a/src/core/ddsi/src/q_receive.c b/src/core/ddsi/src/q_receive.c index a7d4036..0477aee 100644 --- a/src/core/ddsi/src/q_receive.c +++ b/src/core/ddsi/src/q_receive.c @@ -133,7 +133,7 @@ static int valid_AckNack (AckNack_t *msg, size_t size, int byteswap) submessage, and verify that the submessage is large enough */ if (size < ACKNACK_SIZE (msg->readerSNState.numbits)) return 0; - count = (nn_count_t *) ((char *) &msg->readerSNState + NN_SEQUENCE_NUMBER_SET_SIZE (msg->readerSNState.numbits)); + count = (nn_count_t *) ((char *) &msg->bits + NN_SEQUENCE_NUMBER_SET_BITS_SIZE (msg->readerSNState.numbits)); if (byteswap) { bswap_sequence_number_set_bitmap (&msg->readerSNState, msg->bits); @@ -283,8 +283,7 @@ static int valid_NackFrag (NackFrag_t *msg, size_t size, int byteswap) submessage, and verify that the submessage is large enough */ if (size < NACKFRAG_SIZE (msg->fragmentNumberState.numbits)) return 0; - count = (nn_count_t *) ((char *) &msg->fragmentNumberState + - NN_FRAGMENT_NUMBER_SET_SIZE (msg->fragmentNumberState.numbits)); + count = (nn_count_t *) ((char *) &msg->bits + NN_FRAGMENT_NUMBER_SET_BITS_SIZE (msg->fragmentNumberState.numbits)); if (byteswap) { bswap_fragment_number_set_bitmap (&msg->fragmentNumberState, msg->bits); @@ -651,8 +650,7 @@ static int handle_AckNack (struct receiver_state *rst, nn_etime_t tnow, const Ac struct whc_state whcst; int hb_sent_in_response = 0; memset (gapbits, 0, sizeof (gapbits)); - countp = (nn_count_t *) ((char *) msg + offsetof (AckNack_t, readerSNState) + - NN_SEQUENCE_NUMBER_SET_SIZE (msg->readerSNState.numbits)); + countp = (nn_count_t *) ((char *) msg + offsetof (AckNack_t, bits) + NN_SEQUENCE_NUMBER_SET_BITS_SIZE (msg->readerSNState.numbits)); src.prefix = rst->src_guid_prefix; src.entityid = msg->readerId; dst.prefix = rst->dst_guid_prefix; @@ -1382,7 +1380,7 @@ static int handle_NackFrag (struct receiver_state *rst, nn_etime_t tnow, const N nn_count_t *countp; seqno_t seq = fromSN (msg->writerSN); - countp = (nn_count_t *) ((char *) msg + offsetof (NackFrag_t, fragmentNumberState) + NN_FRAGMENT_NUMBER_SET_SIZE (msg->fragmentNumberState.numbits)); + countp = (nn_count_t *) ((char *) msg + offsetof (NackFrag_t, bits) + NN_FRAGMENT_NUMBER_SET_BITS_SIZE (msg->fragmentNumberState.numbits)); src.prefix = rst->src_guid_prefix; src.entityid = msg->readerId; dst.prefix = rst->dst_guid_prefix; diff --git a/src/core/ddsi/src/q_xevent.c b/src/core/ddsi/src/q_xevent.c index 4588c0d..d4de160 100644 --- a/src/core/ddsi/src/q_xevent.c +++ b/src/core/ddsi/src/q_xevent.c @@ -699,7 +699,7 @@ static void add_AckNack (struct nn_xmsg *msg, struct proxy_writer *pwr, struct p seqno_t base; DDSRT_STATIC_ASSERT ((NN_FRAGMENT_NUMBER_SET_MAX_BITS % 32) == 0); - union { + struct { struct nn_fragment_number_set_header set; uint32_t bits[NN_FRAGMENT_NUMBER_SET_MAX_BITS / 32]; } nackfrag; @@ -788,8 +788,7 @@ static void add_AckNack (struct nn_xmsg *msg, struct proxy_writer *pwr, struct p { /* Count field is at a variable offset ... silly DDSI spec. */ nn_count_t *countp = - (nn_count_t *) ((char *) an + offsetof (AckNack_t, readerSNState) + - NN_SEQUENCE_NUMBER_SET_SIZE (an->readerSNState.numbits)); + (nn_count_t *) ((char *) an + offsetof (AckNack_t, bits) + NN_SEQUENCE_NUMBER_SET_BITS_SIZE (an->readerSNState.numbits)); *countp = ++rwn->count; /* Reset submessage size, now that we know the real size, and update @@ -824,7 +823,7 @@ static void add_AckNack (struct nn_xmsg *msg, struct proxy_writer *pwr, struct p { nn_count_t *countp = - (nn_count_t *) ((char *) nf + offsetof (NackFrag_t, fragmentNumberState) + NN_FRAGMENT_NUMBER_SET_SIZE (nf->fragmentNumberState.numbits)); + (nn_count_t *) ((char *) nf + offsetof (NackFrag_t, bits) + NN_FRAGMENT_NUMBER_SET_BITS_SIZE (nf->fragmentNumberState.numbits)); *countp = ++pwr->nackfragcount; nn_xmsg_submsg_setnext (msg, sm_marker);