From 3da21315f2f3a3578a714729339a97fb528ec291 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Mon, 4 Nov 2019 19:56:53 +0100 Subject: [PATCH] Improve errors for invalid port numbers (#303) Signed-off-by: Erik Boasson --- docs/manual/options.md | 4 +- etc/cyclonedds.rnc | 4 +- etc/cyclonedds.xsd | 4 +- src/core/ddsi/CMakeLists.txt | 2 + .../ddsi/include/dds/ddsi/ddsi_portmapping.h | 47 ++++++ src/core/ddsi/include/dds/ddsi/ddsi_tran.h | 15 +- src/core/ddsi/include/dds/ddsi/q_config.h | 9 +- src/core/ddsi/include/dds/ddsi/q_debmon.h | 2 +- src/core/ddsi/src/ddsi_portmapping.c | 126 ++++++++++++++++ src/core/ddsi/src/ddsi_raweth.c | 7 + src/core/ddsi/src/ddsi_tcp.c | 9 +- src/core/ddsi/src/ddsi_tran.c | 3 +- src/core/ddsi/src/ddsi_udp.c | 7 + src/core/ddsi/src/q_addrset.c | 13 +- src/core/ddsi/src/q_config.c | 38 ++--- src/core/ddsi/src/q_debmon.c | 13 +- src/core/ddsi/src/q_init.c | 134 ++++++++++++------ 17 files changed, 337 insertions(+), 100 deletions(-) create mode 100644 src/core/ddsi/include/dds/ddsi/ddsi_portmapping.h create mode 100644 src/core/ddsi/src/ddsi_portmapping.c diff --git a/docs/manual/options.md b/docs/manual/options.md index 25caaf6..6eed7ca 100644 --- a/docs/manual/options.md +++ b/docs/manual/options.md @@ -261,7 +261,7 @@ The default value is: "250". ##### //CycloneDDS/Domain/Discovery/Ports/MulticastDataOffset Integer -This element specifies the port number for multicast meta traffic (refer +This element specifies the port number for multicast data traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d2). The default value is: "1". @@ -289,7 +289,7 @@ The default value is: "2". ##### //CycloneDDS/Domain/Discovery/Ports/UnicastDataOffset Integer -This element specifies the port number for unicast meta traffic (refer to +This element specifies the port number for unicast data traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d3). The default value is: "11". diff --git a/etc/cyclonedds.rnc b/etc/cyclonedds.rnc index 12444a9..95bb3ce 100644 --- a/etc/cyclonedds.rnc +++ b/etc/cyclonedds.rnc @@ -203,7 +203,7 @@ constant DG).

The default value is: "250".

""" ] ] xsd:integer }? & [ a:documentation [ xml:lang="en" """ -

This element specifies the port number for multicast meta traffic +

This element specifies the port number for multicast data traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d2).

The default value is: "1".

""" ] ] element MulticastDataOffset { @@ -225,7 +225,7 @@ section 9.6.1, constant PG).

The default value is: xsd:integer }? & [ a:documentation [ xml:lang="en" """ -

This element specifies the port number for unicast meta traffic (refer +

This element specifies the port number for unicast data traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d3).

The default value is: "11".

""" ] ] element UnicastDataOffset { diff --git a/etc/cyclonedds.xsd b/etc/cyclonedds.xsd index 4e31f2f..4ec63e5 100644 --- a/etc/cyclonedds.xsd +++ b/etc/cyclonedds.xsd @@ -313,7 +313,7 @@ constant DG).</p><p>The default value is: &quot;250&quot;.&l -<p>This element specifies the port number for multicast meta traffic +<p>This element specifies the port number for multicast data traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d2).</p><p>The default value is: &quot;1&quot;.</p> @@ -338,7 +338,7 @@ section 9.6.1, constant PG).</p><p>The default value is: -<p>This element specifies the port number for unicast meta traffic (refer +<p>This element specifies the port number for unicast data traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d3).</p><p>The default value is: &quot;11&quot;.</p> diff --git a/src/core/ddsi/CMakeLists.txt b/src/core/ddsi/CMakeLists.txt index 9e8c6a7..933762a 100644 --- a/src/core/ddsi/CMakeLists.txt +++ b/src/core/ddsi/CMakeLists.txt @@ -18,6 +18,7 @@ PREPEND(srcs_ddsi "${CMAKE_CURRENT_LIST_DIR}/src" ddsi_raweth.c ddsi_ipaddr.c ddsi_mcgroup.c + ddsi_portmapping.c ddsi_serdata.c ddsi_serdata_default.c ddsi_sertopic.c @@ -69,6 +70,7 @@ PREPEND(hdrs_private_ddsi "${CMAKE_CURRENT_LIST_DIR}/include/dds/ddsi" ddsi_ipaddr.h ddsi_mcgroup.h ddsi_plist_generic.h + ddsi_portmapping.h ddsi_serdata.h ddsi_sertopic.h ddsi_serdata_default.h diff --git a/src/core/ddsi/include/dds/ddsi/ddsi_portmapping.h b/src/core/ddsi/include/dds/ddsi/ddsi_portmapping.h new file mode 100644 index 0000000..0374672 --- /dev/null +++ b/src/core/ddsi/include/dds/ddsi/ddsi_portmapping.h @@ -0,0 +1,47 @@ +/* + * Copyright(c) 2019 ADLINK Technology Limited and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, or the Eclipse Distribution License + * v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause + */ +#ifndef DDSI_PORTMAPPING_H +#define DDSI_PORTMAPPING_H + +#include +#include +#include + +#if defined (__cplusplus) +extern "C" { +#endif + +enum ddsi_port { + DDSI_PORT_MULTI_DISC, + DDSI_PORT_MULTI_DATA, + DDSI_PORT_UNI_DISC, + DDSI_PORT_UNI_DATA +}; + +struct ddsi_portmapping { + uint32_t base; + uint32_t dg; + uint32_t pg; + uint32_t d0; + uint32_t d1; + uint32_t d2; + uint32_t d3; +}; + +bool ddsi_valid_portmapping (const struct ddsi_portmapping *map, uint32_t domain_id, int32_t participant_index, char *msg, size_t msgsize); +uint32_t ddsi_get_port (const struct ddsi_portmapping *map, enum ddsi_port which, uint32_t domain_id, int32_t participant_index); + +#if defined (__cplusplus) +} +#endif + +#endif /* DDSI_PORTMAPPING_H */ diff --git a/src/core/ddsi/include/dds/ddsi/ddsi_tran.h b/src/core/ddsi/include/dds/ddsi/ddsi_tran.h index a3e5d89..488b401 100644 --- a/src/core/ddsi/include/dds/ddsi/ddsi_tran.h +++ b/src/core/ddsi/include/dds/ddsi/ddsi_tran.h @@ -61,7 +61,7 @@ typedef void (*ddsi_tran_peer_locator_fn_t) (ddsi_tran_conn_t, nn_locator_t *); typedef void (*ddsi_tran_disable_multiplexing_fn_t) (ddsi_tran_conn_t); typedef ddsi_tran_conn_t (*ddsi_tran_accept_fn_t) (ddsi_tran_listener_t); typedef ddsi_tran_conn_t (*ddsi_tran_create_conn_fn_t) (ddsi_tran_factory_t fact, uint32_t, ddsi_tran_qos_t); -typedef ddsi_tran_listener_t (*ddsi_tran_create_listener_fn_t) (ddsi_tran_factory_t fact, int port, ddsi_tran_qos_t); +typedef ddsi_tran_listener_t (*ddsi_tran_create_listener_fn_t) (ddsi_tran_factory_t fact, uint32_t port, ddsi_tran_qos_t); typedef void (*ddsi_tran_release_conn_fn_t) (ddsi_tran_conn_t); typedef void (*ddsi_tran_close_conn_fn_t) (ddsi_tran_conn_t); typedef void (*ddsi_tran_unblock_listener_fn_t) (ddsi_tran_listener_t); @@ -70,6 +70,7 @@ typedef int (*ddsi_tran_join_mc_fn_t) (ddsi_tran_conn_t, const nn_locator_t *src typedef int (*ddsi_tran_leave_mc_fn_t) (ddsi_tran_conn_t, const nn_locator_t *srcip, const nn_locator_t *mcip, const struct nn_interface *interf); typedef int (*ddsi_is_mcaddr_fn_t) (ddsi_tran_factory_t tran, const nn_locator_t *loc); typedef int (*ddsi_is_ssm_mcaddr_fn_t) (ddsi_tran_factory_t tran, const nn_locator_t *loc); +typedef int (*ddsi_is_valid_port_fn_t) (ddsi_tran_factory_t tran, uint32_t port); enum ddsi_nearby_address_result { DNAR_DISTANT, @@ -172,7 +173,8 @@ struct ddsi_tran_factory ddsi_locator_from_string_fn_t m_locator_from_string_fn; ddsi_locator_to_string_fn_t m_locator_to_string_fn; ddsi_enumerate_interfaces_fn_t m_enumerate_interfaces_fn; - + ddsi_is_valid_port_fn_t m_is_valid_port_fn; + /* Data */ int32_t m_kind; @@ -205,10 +207,17 @@ void ddsi_factory_conn_init (const struct ddsi_tran_factory *factory, ddsi_tran_ inline bool ddsi_factory_supports (const struct ddsi_tran_factory *factory, int32_t kind) { return factory->m_supports_fn (factory, kind); } +inline int ddsi_is_valid_port (ddsi_tran_factory_t factory, uint32_t port) { + return factory->m_is_valid_port_fn (factory, port); +} inline ddsi_tran_conn_t ddsi_factory_create_conn (ddsi_tran_factory_t factory, uint32_t port, ddsi_tran_qos_t qos) { + if (!ddsi_is_valid_port (factory, port)) + return NULL; return factory->m_create_conn_fn (factory, port, qos); } -inline ddsi_tran_listener_t ddsi_factory_create_listener (ddsi_tran_factory_t factory, int port, ddsi_tran_qos_t qos) { +inline ddsi_tran_listener_t ddsi_factory_create_listener (ddsi_tran_factory_t factory, uint32_t port, ddsi_tran_qos_t qos) { + if (!ddsi_is_valid_port (factory, port)) + return NULL; return factory->m_create_listener_fn (factory, port, qos); } diff --git a/src/core/ddsi/include/dds/ddsi/q_config.h b/src/core/ddsi/include/dds/ddsi/q_config.h index 02b26c1..e4cebcd 100644 --- a/src/core/ddsi/include/dds/ddsi/q_config.h +++ b/src/core/ddsi/include/dds/ddsi/q_config.h @@ -16,6 +16,7 @@ #include "dds/ddsi/q_thread.h" #include "dds/ddsi/q_xqos.h" #include "dds/ddsi/q_feature_check.h" +#include "dds/ddsi/ddsi_portmapping.h" #if defined (__cplusplus) extern "C" { @@ -198,7 +199,6 @@ struct config uint32_t domainId; int participantIndex; int maxAutoParticipantIndex; - uint32_t port_base; char *spdpMulticastAddressString; char *defaultMulticastAddressString; char *assumeMulticastCapable; @@ -321,12 +321,7 @@ struct config enum many_sockets_mode many_sockets_mode; int assume_rti_has_pmd_endpoints; - uint32_t port_dg; - uint32_t port_pg; - uint32_t port_d0; - uint32_t port_d1; - uint32_t port_d2; - uint32_t port_d3; + struct ddsi_portmapping ports; int monitor_port; diff --git a/src/core/ddsi/include/dds/ddsi/q_debmon.h b/src/core/ddsi/include/dds/ddsi/q_debmon.h index defb83e..5f5ac06 100644 --- a/src/core/ddsi/include/dds/ddsi/q_debmon.h +++ b/src/core/ddsi/include/dds/ddsi/q_debmon.h @@ -20,7 +20,7 @@ struct debug_monitor; typedef int (*debug_monitor_cpf_t) (ddsi_tran_conn_t conn, const char *fmt, ...); typedef int (*debug_monitor_plugin_t) (ddsi_tran_conn_t conn, debug_monitor_cpf_t cpf, void *arg); -struct debug_monitor *new_debug_monitor (struct q_globals *gv, int port); +struct debug_monitor *new_debug_monitor (struct q_globals *gv, int32_t port); void add_debug_monitor_plugin (struct debug_monitor *dm, debug_monitor_plugin_t fn, void *arg); void free_debug_monitor (struct debug_monitor *dm); diff --git a/src/core/ddsi/src/ddsi_portmapping.c b/src/core/ddsi/src/ddsi_portmapping.c new file mode 100644 index 0000000..164ebfe --- /dev/null +++ b/src/core/ddsi/src/ddsi_portmapping.c @@ -0,0 +1,126 @@ +/* + * Copyright(c) 2019 ADLINK Technology Limited and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, or the Eclipse Distribution License + * v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause + */ +#include +#include "dds/ddsi/ddsi_portmapping.h" +#include "dds/ddsi/q_config.h" + +static bool get_port_int (uint32_t *port, const struct ddsi_portmapping *map, enum ddsi_port which, uint32_t domain_id, int32_t participant_index, char *str_if_overflow, size_t strsize) +{ + uint32_t off = UINT32_MAX, ppidx = UINT32_MAX; + + assert (domain_id != UINT32_MAX); + assert (participant_index >= 0 || participant_index == PARTICIPANT_INDEX_NONE); + + switch (which) + { + case DDSI_PORT_MULTI_DISC: + off = map->d0; + /* multicast port numbers are not affected by participant index */ + ppidx = 0; + break; + case DDSI_PORT_MULTI_DATA: + off = map->d2; + /* multicast port numbers are not affected by participant index */ + ppidx = 0; + break; + case DDSI_PORT_UNI_DISC: + if (participant_index == PARTICIPANT_INDEX_NONE) + { + /* participant index "none" means unicast ports get chosen by the transport */ + *port = 0; + return true; + } + off = map->d1; + ppidx = (uint32_t) participant_index; + break; + case DDSI_PORT_UNI_DATA: + if (participant_index == PARTICIPANT_INDEX_NONE) + { + /* participant index "none" means unicast ports get chosen by the transport */ + *port = 0; + return true; + } + off = map->d3; + ppidx = (uint32_t) participant_index; + break; + } + + const uint64_t a = (uint64_t) map->dg * domain_id; + const uint64_t b = map->base + (uint64_t) map->pg * ppidx + off; + + /* For the mapping to be valid, the port number must be in range of an unsigned 32 bit integer and must + not be 0 (as that is used for indicating a random port should be selected by the transport). The + transports may limit this further, but at least we won't have to worry about overflow anymore. */ + *port = (uint32_t) (a + b); + if (a <= UINT32_MAX && b <= UINT32_MAX - a && *port > 0) + return true; + else + { + /* a, b < 2^64 ~ 18e18; 2^32 <= a + b < 2^65 ~ 36e18 + 2^32 ~ 4e9, so it can easily be split into (a+b) `div` 1e9 and (a+b) `mod` 1e9 + and then the most-significant part is guaranteed to be > 0 */ + const uint32_t billion = 1000000000; + const uint32_t y = (uint32_t) (a % billion) + (uint32_t) (b % billion); + const uint64_t x = (a / billion) + (b / billion) + (y / billion); + snprintf (str_if_overflow, strsize, "%"PRIu64"%09"PRIu32, x, y % billion); + return false; + } +} + +static const char *portname (enum ddsi_port which) +{ + const char *n = "?"; + switch (which) + { + case DDSI_PORT_MULTI_DISC: n = "multicast discovery"; break; + case DDSI_PORT_MULTI_DATA: n = "multicast data"; break; + case DDSI_PORT_UNI_DISC: n = "unicast discovery"; break; + case DDSI_PORT_UNI_DATA: n = "unicast data"; break; + } + return n; +} + +bool ddsi_valid_portmapping (const struct ddsi_portmapping *map, uint32_t domain_id, int32_t participant_index, char *msg, size_t msgsize) +{ + DDSRT_STATIC_ASSERT (DDSI_PORT_MULTI_DISC >= 0 && + DDSI_PORT_MULTI_DISC + 1 == DDSI_PORT_MULTI_DATA && + DDSI_PORT_MULTI_DATA + 1 == DDSI_PORT_UNI_DISC && + DDSI_PORT_UNI_DISC + 1 == DDSI_PORT_UNI_DATA && + DDSI_PORT_UNI_DATA >= 0); + uint32_t dummy_port; + char str[32]; + bool ok = true; + enum ddsi_port which = DDSI_PORT_MULTI_DISC; + int n = snprintf (msg, msgsize, "port number(s) of out range:"); + size_t pos = (n >= 0 && (size_t) n <= msgsize) ? (size_t) n : msgsize; + do { + if (!get_port_int (&dummy_port, map, which, domain_id, participant_index, str, sizeof (str))) + { + n = snprintf (msg + pos, msgsize - pos, "%s %s %s", ok ? "" : ",", portname (which), str); + if (n >= 0 && (size_t) n <= msgsize - pos) + pos += (size_t) n; + ok = false; + } + } while (which++ != DDSI_PORT_UNI_DATA); + return ok; +} + +uint32_t ddsi_get_port (const struct ddsi_portmapping *map, enum ddsi_port which, uint32_t domain_id, int32_t participant_index) +{ + /* Not supposed to come here if port mapping is invalid */ + uint32_t port; + char str[32]; + bool ok = get_port_int (&port, map, which, domain_id, participant_index, str, sizeof (str)); + assert (ok); + (void) ok; + return port; +} diff --git a/src/core/ddsi/src/ddsi_raweth.c b/src/core/ddsi/src/ddsi_raweth.c index bdb9e27..153fcf7 100644 --- a/src/core/ddsi/src/ddsi_raweth.c +++ b/src/core/ddsi/src/ddsi_raweth.c @@ -356,6 +356,12 @@ static int ddsi_raweth_enumerate_interfaces (ddsi_tran_factory_t fact, enum tran return ddsrt_getifaddrs(ifs, afs); } +static int ddsi_raweth_is_valid_port (ddsi_tran_factory_t fact, uint32_t port) +{ + (void) fact; + return (port >= 1 && port <= 65535); +} + int ddsi_raweth_init (struct q_globals *gv) { struct ddsi_tran_factory *fact = ddsrt_malloc (sizeof (*fact)); @@ -377,6 +383,7 @@ int ddsi_raweth_init (struct q_globals *gv) fact->m_locator_from_string_fn = ddsi_raweth_address_from_string; fact->m_locator_to_string_fn = ddsi_raweth_to_string; fact->m_enumerate_interfaces_fn = ddsi_raweth_enumerate_interfaces; + fact->m_is_valid_port_fn = ddsi_raweth_is_valid_port; ddsi_factory_add (gv, fact); GVLOG (DDS_LC_CONFIG, "raweth initialized\n"); return 0; diff --git a/src/core/ddsi/src/ddsi_tcp.c b/src/core/ddsi/src/ddsi_tcp.c index 778d733..47ff028 100644 --- a/src/core/ddsi/src/ddsi_tcp.c +++ b/src/core/ddsi/src/ddsi_tcp.c @@ -856,7 +856,7 @@ static ddsi_tcp_conn_t ddsi_tcp_new_conn (struct ddsi_tran_factory_tcp *fact, dd return conn; } -static ddsi_tran_listener_t ddsi_tcp_create_listener (ddsi_tran_factory_t fact, int port, ddsi_tran_qos_t qos) +static ddsi_tran_listener_t ddsi_tcp_create_listener (ddsi_tran_factory_t fact, uint32_t port, ddsi_tran_qos_t qos) { char buff[DDSI_LOCSTRLEN]; ddsrt_socket_t sock; @@ -1055,6 +1055,12 @@ static enum ddsi_nearby_address_result ddsi_tcp_is_nearby_address (ddsi_tran_fac return ddsi_ipaddr_is_nearby_address(tran, loc, ownloc, ninterf, interf); } +static int ddsi_tcp_is_valid_port (ddsi_tran_factory_t fact, uint32_t port) +{ + (void) fact; + return (port <= 65535); +} + int ddsi_tcp_init (struct q_globals *gv) { struct ddsi_tran_factory_tcp *fact = ddsrt_malloc (sizeof (*fact)); @@ -1079,6 +1085,7 @@ int ddsi_tcp_init (struct q_globals *gv) fact->fact.m_is_mcaddr_fn = ddsi_tcp_is_mcaddr; fact->fact.m_is_ssm_mcaddr_fn = ddsi_tcp_is_ssm_mcaddr; fact->fact.m_is_nearby_address_fn = ddsi_tcp_is_nearby_address; + fact->fact.m_is_valid_port_fn = ddsi_tcp_is_valid_port; ddsi_factory_add (gv, &fact->fact); #if DDSRT_HAVE_IPV6 diff --git a/src/core/ddsi/src/ddsi_tran.c b/src/core/ddsi/src/ddsi_tran.c index 3094d29..7636167 100644 --- a/src/core/ddsi/src/ddsi_tran.c +++ b/src/core/ddsi/src/ddsi_tran.c @@ -23,8 +23,9 @@ extern inline uint32_t ddsi_conn_type (ddsi_tran_conn_t conn); extern inline uint32_t ddsi_conn_port (ddsi_tran_conn_t conn); -extern inline ddsi_tran_listener_t ddsi_factory_create_listener (ddsi_tran_factory_t factory, int port, ddsi_tran_qos_t qos); +extern inline ddsi_tran_listener_t ddsi_factory_create_listener (ddsi_tran_factory_t factory, uint32_t port, ddsi_tran_qos_t qos); extern inline bool ddsi_factory_supports (const struct ddsi_tran_factory *factory, int32_t kind); +extern inline int ddsi_is_valid_port (ddsi_tran_factory_t factory, uint32_t port); extern inline ddsrt_socket_t ddsi_conn_handle (ddsi_tran_conn_t conn); extern inline int ddsi_conn_locator (ddsi_tran_conn_t conn, nn_locator_t * loc); extern inline ddsrt_socket_t ddsi_tran_handle (ddsi_tran_base_t base); diff --git a/src/core/ddsi/src/ddsi_udp.c b/src/core/ddsi/src/ddsi_udp.c index 3d0a93d..90b1843 100644 --- a/src/core/ddsi/src/ddsi_udp.c +++ b/src/core/ddsi/src/ddsi_udp.c @@ -466,6 +466,12 @@ static void ddsi_udp_fini (ddsi_tran_factory_t fact) ddsrt_free (fact); } +static int ddsi_udp_is_valid_port (ddsi_tran_factory_t fact, uint32_t port) +{ + (void) fact; + return (port <= 65535); +} + int ddsi_udp_init (struct q_globals *gv) { struct ddsi_tran_factory *fact = ddsrt_malloc (sizeof (*fact)); @@ -489,6 +495,7 @@ int ddsi_udp_init (struct q_globals *gv) fact->m_locator_from_string_fn = ddsi_udp_address_from_string; fact->m_locator_to_string_fn = ddsi_udp_locator_to_string; fact->m_enumerate_interfaces_fn = ddsi_eth_enumerate_interfaces; + fact->m_is_valid_port_fn = ddsi_udp_is_valid_port; #if DDSRT_HAVE_IPV6 if (gv->config.transport_selector == TRANS_UDP6) { diff --git a/src/core/ddsi/src/q_addrset.c b/src/core/ddsi/src/q_addrset.c index b04f064..7bc6d59 100644 --- a/src/core/ddsi/src/q_addrset.c +++ b/src/core/ddsi/src/q_addrset.c @@ -103,25 +103,22 @@ static int add_addresses_to_addrset_1 (const struct q_globals *gv, struct addrse if (!ddsi_is_mcaddr (gv, &loc)) { assert (gv->config.maxAutoParticipantIndex >= 0); - for (uint32_t i = 0; i <= (uint32_t) gv->config.maxAutoParticipantIndex; i++) + for (int32_t i = 0; i <= gv->config.maxAutoParticipantIndex; i++) { - uint32_t port = gv->config.port_base + gv->config.port_dg * gv->config.domainId + i * gv->config.port_pg + gv->config.port_d1; - loc.port = (unsigned) port; + loc.port = ddsi_get_port (&gv->config.ports, DDSI_PORT_UNI_DISC, gv->config.domainId, i); if (i == 0) GVLOG (DDS_LC_CONFIG, "%s", ddsi_locator_to_string(gv, buf, sizeof(buf), &loc)); else - GVLOG (DDS_LC_CONFIG, ", :%"PRIu32, port); + GVLOG (DDS_LC_CONFIG, ", :%"PRIu32, loc.port); add_to_addrset (gv, as, &loc); } } else { - uint32_t port; if (port_mode == -1) - port = gv->config.port_base + gv->config.port_dg * gv->config.domainId + gv->config.port_d0; + loc.port = ddsi_get_port (&gv->config.ports, DDSI_PORT_MULTI_DISC, gv->config.domainId, 0); else - port = (uint32_t) port_mode; - loc.port = (unsigned) port; + loc.port = (uint32_t) port_mode; GVLOG (DDS_LC_CONFIG, "%s", ddsi_locator_to_string(gv, buf, sizeof(buf), &loc)); add_to_addrset (gv, as, &loc); } diff --git a/src/core/ddsi/src/q_config.c b/src/core/ddsi/src/q_config.c index 75ca4fe..481c767 100644 --- a/src/core/ddsi/src/q_config.c +++ b/src/core/ddsi/src/q_config.c @@ -166,7 +166,6 @@ DUPF(uint32); DU(natint); DU(natint_255); DUPF(participantIndex); -DU(port); DU(dyn_port); DUPF(memsize); DU(duration_inf); @@ -610,20 +609,20 @@ static const struct cfgelem sizing_cfgelems[] = { }; static const struct cfgelem discovery_ports_cfgelems[] = { - { LEAF("Base"), 1, "7400", ABSOFF(port_base), 0, uf_port, 0, pf_uint, + { LEAF("Base"), 1, "7400", ABSOFF(ports.base), 0, uf_uint, 0, pf_uint, BLURB("

This element specifies the base port number (refer to the DDSI 2.1 specification, section 9.6.1, constant PB).

") }, - { LEAF("DomainGain"), 1, "250", ABSOFF(port_dg), 0, uf_uint, 0, pf_uint, + { LEAF("DomainGain"), 1, "250", ABSOFF(ports.dg), 0, uf_uint, 0, pf_uint, BLURB("

This element specifies the domain gain, relating domain ids to sets of port numbers (refer to the DDSI 2.1 specification, section 9.6.1, constant DG).

") }, - { LEAF("ParticipantGain"), 1, "2", ABSOFF(port_pg), 0, uf_uint, 0, pf_uint, + { LEAF("ParticipantGain"), 1, "2", ABSOFF(ports.pg), 0, uf_uint, 0, pf_uint, BLURB("

This element specifies the participant gain, relating p0, articipant index to sets of port numbers (refer to the DDSI 2.1 specification, section 9.6.1, constant PG).

") }, - { LEAF("MulticastMetaOffset"), 1, "0", ABSOFF(port_d0), 0, uf_uint, 0, pf_uint, + { LEAF("MulticastMetaOffset"), 1, "0", ABSOFF(ports.d0), 0, uf_uint, 0, pf_uint, BLURB("

This element specifies the port number for multicast meta traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d0).

") }, - { LEAF("UnicastMetaOffset"), 1, "10", ABSOFF(port_d1), 0, uf_uint, 0, pf_uint, + { LEAF("UnicastMetaOffset"), 1, "10", ABSOFF(ports.d1), 0, uf_uint, 0, pf_uint, BLURB("

This element specifies the port number for unicast meta traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d1).

") }, - { LEAF("MulticastDataOffset"), 1, "1", ABSOFF(port_d2), 0, uf_uint, 0, pf_uint, - BLURB("

This element specifies the port number for multicast meta traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d2).

") }, - { LEAF("UnicastDataOffset"), 1, "11", ABSOFF(port_d3), 0, uf_uint, 0, pf_uint, - BLURB("

This element specifies the port number for unicast meta traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d3).

") }, + { LEAF("MulticastDataOffset"), 1, "1", ABSOFF(ports.d2), 0, uf_uint, 0, pf_uint, + BLURB("

This element specifies the port number for multicast data traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d2).

") }, + { LEAF("UnicastDataOffset"), 1, "11", ABSOFF(ports.d3), 0, uf_uint, 0, pf_uint, + BLURB("

This element specifies the port number for unicast data traffic (refer to the DDSI 2.1 specification, section 9.6.1, constant d3).

") }, END_MARKER }; @@ -1883,34 +1882,23 @@ static enum update_result uf_natint_255(struct cfgst *cfgst, void *parent, struc static enum update_result uf_uint (struct cfgst *cfgst, void *parent, struct cfgelem const * const cfgelem, UNUSED_ARG (int first), const char *value) { - unsigned * const elem = cfg_address (cfgst, parent, cfgelem); + uint32_t * const elem = cfg_address (cfgst, parent, cfgelem); char *endptr; unsigned long v = strtoul (value, &endptr, 10); if (*value == 0 || *endptr != 0) return cfg_error (cfgst, "%s: not a decimal integer", value); - if (v != (unsigned) v) + if (v != (uint32_t) v) return cfg_error (cfgst, "%s: value out of range", value); - *elem = (unsigned) v; + *elem = (uint32_t) v; return URES_SUCCESS; } static void pf_uint (struct cfgst *cfgst, void *parent, struct cfgelem const * const cfgelem, uint32_t sources) { - unsigned const * const p = cfg_address (cfgst, parent, cfgelem); + uint32_t const * const p = cfg_address (cfgst, parent, cfgelem); cfg_logelem (cfgst, sources, "%u", *p); } -static enum update_result uf_port(struct cfgst *cfgst, void *parent, struct cfgelem const * const cfgelem, int first, const char *value) -{ - int *elem = cfg_address (cfgst, parent, cfgelem); - if (uf_uint (cfgst, parent, cfgelem, first, value) != URES_SUCCESS) - return URES_ERROR; - else if (*elem < 1 || *elem > 65535) - return cfg_error (cfgst, "%s: out of range", value); - else - return URES_SUCCESS; -} - static enum update_result uf_duration_gen (struct cfgst *cfgst, void *parent, struct cfgelem const * const cfgelem, const char *value, int64_t def_mult, int64_t min_ns, int64_t max_ns) { return uf_natint64_unit (cfgst, cfg_address (cfgst, parent, cfgelem), value, unittab_duration, def_mult, min_ns, max_ns); diff --git a/src/core/ddsi/src/q_debmon.c b/src/core/ddsi/src/q_debmon.c index 58c8971..b4f6c29 100644 --- a/src/core/ddsi/src/q_debmon.c +++ b/src/core/ddsi/src/q_debmon.c @@ -346,10 +346,11 @@ static uint32_t debmon_main (void *vdm) return 0; } -struct debug_monitor *new_debug_monitor (struct q_globals *gv, int port) +struct debug_monitor *new_debug_monitor (struct q_globals *gv, int32_t port) { struct debug_monitor *dm; + /* negative port number means the feature is disabled */ if (gv->config.monitor_port < 0) return NULL; @@ -362,7 +363,14 @@ struct debug_monitor *new_debug_monitor (struct q_globals *gv, int port) dm->plugins = NULL; if ((dm->tran_factory = ddsi_factory_find (gv, "tcp")) == NULL) dm->tran_factory = ddsi_factory_find (gv, "tcp6"); - dm->servsock = ddsi_factory_create_listener (dm->tran_factory, port, NULL); + + if (!ddsi_is_valid_port (dm->tran_factory, (uint32_t) port)) + { + GVERROR ("debug monitor port number %"PRId32" is invalid\n", port); + goto err_invalid_port; + } + + dm->servsock = ddsi_factory_create_listener (dm->tran_factory, (uint32_t) port, NULL); if (dm->servsock == NULL) { GVWARNING ("debmon: can't create socket\n"); @@ -389,6 +397,7 @@ err_listen: ddsrt_mutex_destroy(&dm->lock); ddsi_listener_free(dm->servsock); err_servsock: +err_invalid_port: ddsrt_free(dm); return NULL; } diff --git a/src/core/ddsi/src/q_init.c b/src/core/ddsi/src/q_init.c index c73a39e..04d94ba 100644 --- a/src/core/ddsi/src/q_init.c +++ b/src/core/ddsi/src/q_init.c @@ -71,36 +71,30 @@ static void add_peer_addresses (const struct q_globals *gv, struct addrset *as, } } -static int make_uc_sockets (struct q_globals *gv, uint32_t * pdisc, uint32_t * pdata, int ppid) +enum make_uc_sockets_ret { + MUSRET_SUCCESS, + MUSRET_INVALID_PORTS, + MUSRET_NOSOCKET +}; + +static enum make_uc_sockets_ret make_uc_sockets (struct q_globals *gv, uint32_t * pdisc, uint32_t * pdata, int ppid) { if (gv->config.many_sockets_mode == MSM_NO_UNICAST) { assert (ppid == PARTICIPANT_INDEX_NONE); - *pdata = *pdisc = (uint32_t) (gv->config.port_base + gv->config.port_dg * gv->config.domainId); + *pdata = *pdisc = ddsi_get_port (&gv->config.ports, DDSI_PORT_MULTI_DISC, gv->config.domainId, ppid); if (gv->config.allowMulticast) { /* FIXME: ugly hack - but we'll fix up after creating the multicast sockets */ - return 0; + return MUSRET_SUCCESS; } } - if (ppid >= 0) - { - /* FIXME: verify port numbers are in range instead of truncating them like this */ - uint32_t base = gv->config.port_base + (gv->config.port_dg * gv->config.domainId) + ((uint32_t) ppid * gv->config.port_pg); - *pdisc = base + gv->config.port_d1; - *pdata = base + gv->config.port_d3; - } - else if (ppid == PARTICIPANT_INDEX_NONE) - { - *pdata = 0; - *pdisc = 0; - } - else - { - DDS_FATAL ("make_uc_sockets: invalid participant index %d\n", ppid); - return -1; - } + *pdisc = ddsi_get_port (&gv->config.ports, DDSI_PORT_UNI_DISC, gv->config.domainId, ppid); + *pdata = ddsi_get_port (&gv->config.ports, DDSI_PORT_UNI_DATA, gv->config.domainId, ppid); + + if (!ddsi_is_valid_port (gv->m_factory, *pdisc) || !ddsi_is_valid_port (gv->m_factory, *pdata)) + return MUSRET_INVALID_PORTS; gv->disc_conn_uc = ddsi_factory_create_conn (gv->m_factory, *pdisc, NULL); if (gv->disc_conn_uc) @@ -123,13 +117,12 @@ static int make_uc_sockets (struct q_globals *gv, uint32_t * pdisc, uint32_t * p else { /* Set unicast locators */ - ddsi_conn_locator (gv->disc_conn_uc, &gv->loc_meta_uc); ddsi_conn_locator (gv->data_conn_uc, &gv->loc_default_uc); } } - return gv->data_conn_uc ? 0 : -1; + return gv->data_conn_uc ? MUSRET_SUCCESS : MUSRET_NOSOCKET; } static void make_builtin_endpoint_xqos (dds_qos_t *q, const dds_qos_t *template) @@ -286,7 +279,7 @@ static int string_to_default_locator (const struct q_globals *gv, nn_locator_t * static int set_spdp_address (struct q_globals *gv) { - const uint32_t port = (uint32_t) (gv->config.port_base + gv->config.port_dg * gv->config.domainId + gv->config.port_d0); + const uint32_t port = ddsi_get_port (&gv->config.ports, DDSI_PORT_MULTI_DISC, gv->config.domainId, 0); int rc = 0; /* FIXME: FIXME: FIXME: */ gv->loc_spdp_mc.kind = NN_LOCATOR_KIND_INVALID; @@ -318,7 +311,7 @@ static int set_spdp_address (struct q_globals *gv) static int set_default_mc_address (struct q_globals *gv) { - const uint32_t port = (uint32_t) (gv->config.port_base + gv->config.port_dg * gv->config.domainId + gv->config.port_d2); + const uint32_t port = ddsi_get_port (&gv->config.ports, DDSI_PORT_MULTI_DATA, gv->config.domainId, 0); int rc; if (!gv->config.defaultMulticastAddressString) gv->loc_default_mc = gv->loc_spdp_mc; @@ -459,6 +452,25 @@ int rtps_config_prep (struct q_globals *gv, struct cfgst *cfgst) unsigned num_channel_threads = 0; #endif + { + char message[256]; + int32_t ppidx; + if (gv->config.participantIndex >= 0 || gv->config.participantIndex == PARTICIPANT_INDEX_NONE) + ppidx = gv->config.participantIndex; + else if (gv->config.participantIndex == PARTICIPANT_INDEX_AUTO) + ppidx = gv->config.maxAutoParticipantIndex; + else + { + assert (0); + ppidx = 0; + } + if (!ddsi_valid_portmapping (&gv->config.ports, gv->config.domainId, ppidx, message, sizeof (message))) + { + DDS_ILOG (DDS_LC_ERROR, gv->config.domainId, "Invalid port mapping: %s\n", message); + goto err_config_late_error; + } + } + /* retry_on_reject_duration default is dependent on late_ack_mode and responsiveness timeout, so fix up */ if (gv->config.whc_init_highwater_mark.isdefault) gv->config.whc_init_highwater_mark.value = gv->config.whc_lowwater_mark; @@ -646,8 +658,13 @@ int create_multicast_sockets (struct q_globals *gv) uint32_t port; qos->m_multicast = 1; - /* FIXME: should check for overflow */ - port = (uint32_t) (gv->config.port_base + gv->config.port_dg * gv->config.domainId + gv->config.port_d0); + port = ddsi_get_port (&gv->config.ports, DDSI_PORT_MULTI_DISC, gv->config.domainId, 0); + if (!ddsi_is_valid_port (gv->m_factory, port)) + { + GVERROR ("Failed to create discovery multicast socket for domain %"PRIu32": resulting port number (%"PRIu32") is out of range\n", + gv->config.domainId, port); + goto err_disc; + } if ((disc = ddsi_factory_create_conn (gv->m_factory, port, qos)) == NULL) goto err_disc; if (gv->config.many_sockets_mode == MSM_NO_UNICAST) @@ -657,16 +674,24 @@ int create_multicast_sockets (struct q_globals *gv) } else { - port = (uint32_t) (gv->config.port_base + gv->config.port_dg * gv->config.domainId + gv->config.port_d2); + port = ddsi_get_port (&gv->config.ports, DDSI_PORT_MULTI_DATA, gv->config.domainId, 0); + if (!ddsi_is_valid_port (gv->m_factory, port)) + { + GVERROR ("Failed to create data multicast socket for domain %"PRIu32": resulting port number (%"PRIu32") is out of range\n", + gv->config.domainId, port); + goto err_disc; + } if ((data = ddsi_factory_create_conn (gv->m_factory, port, qos)) == NULL) + { goto err_data; + } } ddsi_tran_free_qos (qos); gv->disc_conn_mc = disc; gv->data_conn_mc = data; - GVTRACE ("Multicast Ports: discovery %"PRIu32" data %"PRIu32" \n", - ddsi_conn_port (gv->disc_conn_mc), ddsi_conn_port (gv->data_conn_mc)); + GVLOG (DDS_LC_CONFIG, "Multicast Ports: discovery %"PRIu32" data %"PRIu32" \n", + ddsi_conn_port (gv->disc_conn_mc), ddsi_conn_port (gv->data_conn_mc)); return 1; err_data: @@ -980,7 +1005,7 @@ int rtps_init (struct q_globals *gv) #ifdef DDSI_INCLUDE_NETWORK_PARTITIONS /* Convert address sets in partition mappings from string to address sets */ { - const uint32_t port = gv->config.port_base + gv->config.port_dg * gv->config.domainId + gv->config.port_d2; + const uint32_t port = ddsi_get_port (&gv->config.ports, DDSI_PORT_MULTI_DATA, gv->config.domainId, 0); struct config_networkpartition_listelem *np; for (np = gv->config.networkPartitions; np; np = np->next) { @@ -1070,33 +1095,44 @@ int rtps_init (struct q_globals *gv) { if (gv->config.participantIndex >= 0 || gv->config.participantIndex == PARTICIPANT_INDEX_NONE) { - if (make_uc_sockets (gv, &port_disc_uc, &port_data_uc, gv->config.participantIndex) < 0) + enum make_uc_sockets_ret musret = make_uc_sockets (gv, &port_disc_uc, &port_data_uc, gv->config.participantIndex); + switch (musret) { - GVERROR ("rtps_init: failed to create unicast sockets for domain %"PRId32" participant %d\n", gv->config.domainId, gv->config.participantIndex); - goto err_unicast_sockets; + case MUSRET_SUCCESS: + break; + case MUSRET_INVALID_PORTS: + GVERROR ("Failed to create unicast sockets for domain %"PRIu32" participant index %d: resulting port numbers (%"PRIu32", %"PRIu32") are out of range\n", + gv->config.domainId, gv->config.participantIndex, port_disc_uc, port_data_uc); + goto err_unicast_sockets; + case MUSRET_NOSOCKET: + GVERROR ("rtps_init: failed to create unicast sockets for domain %"PRId32" participant index %d (ports %"PRIu32", %"PRIu32")\n", gv->config.domainId, gv->config.participantIndex, port_disc_uc, port_data_uc); + goto err_unicast_sockets; } } else if (gv->config.participantIndex == PARTICIPANT_INDEX_AUTO) { /* try to find a free one, and update gv->config.participantIndex */ + enum make_uc_sockets_ret musret = MUSRET_NOSOCKET; int ppid; GVLOG (DDS_LC_CONFIG, "rtps_init: trying to find a free participant index\n"); - for (ppid = 0; ppid <= gv->config.maxAutoParticipantIndex; ppid++) + for (ppid = 0; ppid <= gv->config.maxAutoParticipantIndex && musret == MUSRET_NOSOCKET; ppid++) { - int r = make_uc_sockets (gv, &port_disc_uc, &port_data_uc, ppid); - if (r == 0) /* Success! */ - break; - else if (r == -1) /* Try next one */ - continue; - else /* Oops! */ + musret = make_uc_sockets (gv, &port_disc_uc, &port_data_uc, ppid); + switch (musret) { - GVERROR ("rtps_init: failed to create unicast sockets for domain %"PRId32" participant %d\n", gv->config.domainId, ppid); - goto err_unicast_sockets; + case MUSRET_SUCCESS: + break; + case MUSRET_INVALID_PORTS: + GVERROR ("Failed to create unicast sockets for domain %"PRIu32" participant index %d: resulting port numbers (%"PRIu32", %"PRIu32") are out of range\n", + gv->config.domainId, ppid, port_disc_uc, port_data_uc); + goto err_unicast_sockets; + case MUSRET_NOSOCKET: /* Try next one */ + break; } } if (ppid > gv->config.maxAutoParticipantIndex) { - GVERROR ("rtps_init: failed to find a free participant index for domain %"PRId32"\n", gv->config.domainId); + GVERROR ("Failed to find a free participant index for domain %"PRIu32"\n", gv->config.domainId); goto err_unicast_sockets; } gv->config.participantIndex = ppid; @@ -1107,7 +1143,7 @@ int rtps_init (struct q_globals *gv) } GVLOG (DDS_LC_CONFIG, "rtps_init: uc ports: disc %"PRIu32" data %"PRIu32"\n", port_disc_uc, port_data_uc); } - GVLOG (DDS_LC_CONFIG, "rtps_init: domainid %"PRId32" participantid %d\n", gv->config.domainId, gv->config.participantIndex); + GVLOG (DDS_LC_CONFIG, "rtps_init: domainid %"PRIu32" participantid %d\n", gv->config.domainId, gv->config.participantIndex); if (gv->config.pcap_file && *gv->config.pcap_file) { @@ -1157,9 +1193,15 @@ int rtps_init (struct q_globals *gv) /* Must have a data_conn_uc/tev_conn/transmit_conn */ gv->data_conn_uc = ddsi_factory_create_conn (gv->m_factory, 0, NULL); - if (gv->config.tcp_port != -1) + if (gv->config.tcp_port == -1) + ; /* nop */ + else if (!ddsi_is_valid_port (gv->m_factory, (uint32_t) gv->config.tcp_port)) { - gv->listener = ddsi_factory_create_listener (gv->m_factory, gv->config.tcp_port, NULL); + GVERROR ("Listener port %d is out of range for transport %s\n", gv->config.tcp_port, gv->m_factory->m_typename); + } + else + { + gv->listener = ddsi_factory_create_listener (gv->m_factory, (uint32_t) gv->config.tcp_port, NULL); if (gv->listener == NULL || ddsi_listener_listen (gv->listener) != 0) { GVERROR ("Failed to create %s listener\n", gv->m_factory->m_typename);