Skip to content

Commit

Permalink
Fix get_matched_publication_data on builtin topic
Browse files Browse the repository at this point in the history
This crashed because the built-in topics are published by a "local
orphan writer" that is not contained in a participant, causing
dds_get_matched_publication_data to crash on dereferencing a null
pointer.

Signed-off-by: Erik Boasson <eb@ilities.com>
  • Loading branch information
eboasson committed Oct 11, 2023
1 parent a286d66 commit 43560b9
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 14 deletions.
22 changes: 16 additions & 6 deletions src/core/ddsc/src/dds_matched.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,22 +76,28 @@ static dds_builtintopic_endpoint_t *make_builtintopic_endpoint (const ddsi_guid_
return ep;
}

static const struct ddsi_entity_common null_entity_common;

dds_builtintopic_endpoint_t *dds_get_matched_subscription_data (dds_entity_t writer, dds_instance_handle_t ih)
{
dds_writer *wr;
if (dds_writer_lock (writer, &wr))
return NULL;

dds_builtintopic_endpoint_t *ret = NULL;
struct ddsi_entity_common *rdc;
struct dds_qos *rdqos;
struct ddsi_entity_common *ppc;
const struct ddsi_entity_common *rdc;
const struct dds_qos *rdqos;
const struct ddsi_entity_common *ppc;

// thread must be "awake" while pointers to DDSI entities are being used
struct ddsi_domaingv * const gv = &wr->m_entity.m_domain->gv;
ddsi_thread_state_awake (ddsi_lookup_thread_state (), gv);
if (ddsi_writer_find_matched_reader (wr->m_wr, ih, &rdc, &rdqos, &ppc))
{
if (ppc == NULL)
ppc = &null_entity_common;
ret = make_builtintopic_endpoint (&rdc->guid, &ppc->guid, ppc->iid, rdqos);
}
ddsi_thread_state_asleep (ddsi_lookup_thread_state ());

dds_writer_unlock (wr);
Expand All @@ -105,15 +111,19 @@ dds_builtintopic_endpoint_t *dds_get_matched_publication_data (dds_entity_t read
return NULL;

dds_builtintopic_endpoint_t *ret = NULL;
struct ddsi_entity_common *wrc;
struct dds_qos *wrqos;
struct ddsi_entity_common *ppc;
const struct ddsi_entity_common *wrc;
const struct dds_qos *wrqos;
const struct ddsi_entity_common *ppc;

// thread must be "awake" while pointers to DDSI entities are being used
struct ddsi_domaingv * const gv = &rd->m_entity.m_domain->gv;
ddsi_thread_state_awake (ddsi_lookup_thread_state (), gv);
if (ddsi_reader_find_matched_writer (rd->m_rd, ih, &wrc, &wrqos, &ppc))
{
if (ppc == NULL)
ppc = &null_entity_common;
ret = make_builtintopic_endpoint (&wrc->guid, &ppc->guid, ppc->iid, wrqos);
}
ddsi_thread_state_asleep (ddsi_lookup_thread_state ());

dds_reader_unlock (rd);
Expand Down
30 changes: 26 additions & 4 deletions src/core/ddsi/include/dds/ddsi/ddsi_endpoint_match.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,33 @@ dds_return_t ddsi_writer_get_matched_subscriptions (struct ddsi_writer *wr, uint
/** @component endpoint_matching */
dds_return_t ddsi_reader_get_matched_publications (struct ddsi_reader *rd, uint64_t *wrs, size_t nwrs);

/** @component endpoint_matching */
bool ddsi_writer_find_matched_reader (struct ddsi_writer *wr, uint64_t ih, struct ddsi_entity_common **rdc, struct dds_qos **rdqos, struct ddsi_entity_common **ppc);
/** @brief Lookup the instance handle of a matching reader and return it, it's qos and participant
* @component endpoint_matching
*
* @note Thread state must be "awake", returned pointers are aliases; lifetime ends on leaving the "awake" state; contents may not be modified.
*
* @param[in] wr writer for which to lookup ih in the setting of matching readers
* @param[in] ih instance handle of a reader
* @param[out] rdc pointer to the common entity data of the reader
* @param[out] rdqos pointer to the QoS of the reader
* @param[out] ppc participant, may be null for an orphan reader
* @return true iff ih corresponds to a matched reader, output parameters are undefined if not
*/
bool ddsi_writer_find_matched_reader (struct ddsi_writer *wr, uint64_t ih, const struct ddsi_entity_common **rdc, const struct dds_qos **rdqos, const struct ddsi_entity_common **ppc);

/** @component endpoint_matching */
bool ddsi_reader_find_matched_writer (struct ddsi_reader *rd, uint64_t ih, struct ddsi_entity_common **wrc, struct dds_qos **wrqos, struct ddsi_entity_common **ppc);
/** @brief Lookup the instance handle of a matching writer and return it, it's qos and participant
* @component endpoint_matching
*
* @note Thread state must be "awake", returned pointers are aliases; lifetime ends on leaving the "awake" state; contents may not be modified.
*
* @param[in] rd reader for which to lookup ih in the setting of matching writers
* @param[in] ih instance handle of a writer
* @param[out] wrc pointer to the common entity data of the writer
* @param[out] wrqos pointer to the QoS of the writer
* @param[out] ppc participant, may be null for an orphan writer
* @return true iff ih corresponds to a matched writer, output parameters are undefined if not
*/
bool ddsi_reader_find_matched_writer (struct ddsi_reader *rd, uint64_t ih, const struct ddsi_entity_common **wrc, const struct dds_qos **wrqos, const struct ddsi_entity_common **ppc);

#if defined (__cplusplus)
}
Expand Down
10 changes: 6 additions & 4 deletions src/core/ddsi/src/ddsi_endpoint_match.c
Original file line number Diff line number Diff line change
Expand Up @@ -1864,7 +1864,7 @@ dds_return_t ddsi_reader_get_matched_publications (struct ddsi_reader *rd, uint6
return (dds_return_t) nwrs_act;
}

bool ddsi_writer_find_matched_reader (struct ddsi_writer *wr, uint64_t ih, struct ddsi_entity_common **rdc, struct dds_qos **rdqos, struct ddsi_entity_common **ppc)
bool ddsi_writer_find_matched_reader (struct ddsi_writer *wr, uint64_t ih, const struct ddsi_entity_common **rdc, const struct dds_qos **rdqos, const struct ddsi_entity_common **ppc)
{
/* FIXME: this ought not be so inefficient */
struct ddsi_domaingv *gv = wr->e.gv;
Expand Down Expand Up @@ -1895,14 +1895,15 @@ bool ddsi_writer_find_matched_reader (struct ddsi_writer *wr, uint64_t ih, struc
found = true;
*rdc = &rd->e;
*rdqos = rd->xqos;
*ppc = &rd->c.pp->e;
// The day orphan readers are introduced in addition to orphan writers, rd->c.pp may be a null pointer
*ppc = rd->c.pp ? &rd->c.pp->e : NULL;
}
}
ddsrt_mutex_unlock (&wr->e.lock);
return found;
}

bool ddsi_reader_find_matched_writer (struct ddsi_reader *rd, uint64_t ih, struct ddsi_entity_common **wrc, struct dds_qos **wrqos, struct ddsi_entity_common **ppc)
bool ddsi_reader_find_matched_writer (struct ddsi_reader *rd, uint64_t ih, const struct ddsi_entity_common **wrc, const struct dds_qos **wrqos, const struct ddsi_entity_common **ppc)
{
/* FIXME: this ought not be so inefficient */
struct ddsi_domaingv *gv = rd->e.gv;
Expand Down Expand Up @@ -1933,7 +1934,8 @@ bool ddsi_reader_find_matched_writer (struct ddsi_reader *rd, uint64_t ih, struc
found = true;
*wrc = &wr->e;
*wrqos = wr->xqos;
*ppc = &wr->c.pp->e;
// Orphan writers have no participant
*ppc = wr->c.pp ? &wr->c.pp->e : NULL;
}
}
ddsrt_mutex_unlock (&rd->e.lock);
Expand Down

0 comments on commit 43560b9

Please sign in to comment.