niklas/cyclonedds
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Correct ACKNACK, GAP, NACKFRAG size calculation

Browse source 
Commit 3afce30c37 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 <eb@ilities.com>
This commit is contained in:
Erik Boasson 2019-08-06 14:41:01 +02:00 • committed by eboasson
parent 40ba6b207f
commit 0e83982aeb
3 changed files with 10 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

6
src/core/ddsi/include/dds/ddsi/q_protocol.h
View file

@ -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 {

10
src/core/ddsi/src/q_receive.c
View file

@ -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;

7
src/core/ddsi/src/q_xevent.c
View file

@ -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);