From fe7e672dc5ac37b8f4b7b19450eef78a59d9fd19 Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Tue, 12 Dec 2017 01:29:13 +0800 Subject: [PATCH 1/5] Segmentation error to dereference nullptr getEDPReaders is possible to return nullptr Signed-off-by: Ethan Gao --- rmw_fastrtps_cpp/src/rmw_node.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/rmw_fastrtps_cpp/src/rmw_node.cpp b/rmw_fastrtps_cpp/src/rmw_node.cpp index 537cd49d7..d89aab6c9 100644 --- a/rmw_fastrtps_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_cpp/src/rmw_node.cpp @@ -127,8 +127,13 @@ create_node( node_impl->secondaryPubListener = tnat_2; edp_readers = participant->getEDPReaders(); - if (!(edp_readers.first->setListener(tnat_1) & edp_readers.second->setListener(tnat_2))) { - RMW_SET_ERROR_MSG("Failed to attach ROS related logic to the Participant"); + if (edp_readers.first && edp_readers.second) { + if (!(edp_readers.first->setListener(tnat_1) & edp_readers.second->setListener(tnat_2))) { + RMW_SET_ERROR_MSG("Failed to attach ROS related logic to the Participant"); + goto fail; + } + } else { + RMW_SET_ERROR_MSG("Failed to get valid reader for node subsciber and publisher"); goto fail; } @@ -272,12 +277,12 @@ rmw_destroy_node(rmw_node_t * node) // Begin deleting things in the same order they were created in rmw_create_node(). std::pair edp_readers = participant->getEDPReaders(); - if (!edp_readers.first->setListener(nullptr)) { + if (!edp_readers.first || !edp_readers.first->setListener(nullptr)) { RMW_SET_ERROR_MSG("failed to unset EDPReader listener"); result_ret = RMW_RET_ERROR; } delete impl->secondarySubListener; - if (!edp_readers.second->setListener(nullptr)) { + if (!edp_readers.second || !edp_readers.second->setListener(nullptr)) { RMW_SET_ERROR_MSG("failed to unset EDPReader listener"); result_ret = RMW_RET_ERROR; } From f60f0dd3bb2a652e5fde69c741a111f01e90c230 Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Tue, 12 Dec 2017 01:47:59 +0800 Subject: [PATCH 2/5] Avoid null dereference to application crash Signed-off-by: Ethan Gao --- rmw_fastrtps_cpp/src/rmw_service.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rmw_fastrtps_cpp/src/rmw_service.cpp b/rmw_fastrtps_cpp/src/rmw_service.cpp index 0c85110f8..ec922d6d3 100644 --- a/rmw_fastrtps_cpp/src/rmw_service.cpp +++ b/rmw_fastrtps_cpp/src/rmw_service.cpp @@ -199,6 +199,10 @@ rmw_create_service( } rmw_service = rmw_service_allocate(); + if (!rmw_service) { + RMW_SET_ERROR_MSG("failed to allocate memory for service"); + goto fail; + } rmw_service->implementation_identifier = eprosima_fastrtps_identifier; rmw_service->data = info; rmw_service->service_name = reinterpret_cast( @@ -237,7 +241,7 @@ rmw_create_service( delete info; } - if (rmw_service->service_name != nullptr) { + if (rmw_service && rmw_service->service_name != nullptr) { rmw_free(const_cast(rmw_service->service_name)); rmw_service->service_name = nullptr; } From 8413f3e5a9892c34af200514cbbd4c8b8eaf91f7 Mon Sep 17 00:00:00 2001 From: Ethan Gao <16472154+gaoethan@users.noreply.github.com> Date: Fri, 15 Dec 2017 15:06:14 +0800 Subject: [PATCH 3/5] change the way not equal to nullptr --- rmw_fastrtps_cpp/src/rmw_service.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rmw_fastrtps_cpp/src/rmw_service.cpp b/rmw_fastrtps_cpp/src/rmw_service.cpp index ec922d6d3..43ef1fbaa 100644 --- a/rmw_fastrtps_cpp/src/rmw_service.cpp +++ b/rmw_fastrtps_cpp/src/rmw_service.cpp @@ -241,7 +241,7 @@ rmw_create_service( delete info; } - if (rmw_service && rmw_service->service_name != nullptr) { + if (rmw_service && rmw_service->service_name) { rmw_free(const_cast(rmw_service->service_name)); rmw_service->service_name = nullptr; } From 1b8dd88d12c538d73eb93c4f43a6d9429c2c86dc Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Fri, 15 Dec 2017 23:08:18 +0800 Subject: [PATCH 4/5] reflect the failure to get EDP readers Signed-off-by: Ethan Gao --- rmw_fastrtps_cpp/src/rmw_node.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/rmw_fastrtps_cpp/src/rmw_node.cpp b/rmw_fastrtps_cpp/src/rmw_node.cpp index d89aab6c9..c5f9cead1 100644 --- a/rmw_fastrtps_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_cpp/src/rmw_node.cpp @@ -133,7 +133,7 @@ create_node( goto fail; } } else { - RMW_SET_ERROR_MSG("Failed to get valid reader for node subsciber and publisher"); + RMW_SET_ERROR_MSG("Failed to get valid reader for node subscriber and publisher"); goto fail; } @@ -277,12 +277,17 @@ rmw_destroy_node(rmw_node_t * node) // Begin deleting things in the same order they were created in rmw_create_node(). std::pair edp_readers = participant->getEDPReaders(); - if (!edp_readers.first || !edp_readers.first->setListener(nullptr)) { + if (!edp_readers.first || !edp_readers.second) { + RMW_SET_ERROR_MSG("failed to get EDPReader listener"); + result_ret = RMW_RET_ERROR; + } + + if (edp_readers.first && !edp_readers.first->setListener(nullptr)) { RMW_SET_ERROR_MSG("failed to unset EDPReader listener"); result_ret = RMW_RET_ERROR; } delete impl->secondarySubListener; - if (!edp_readers.second || !edp_readers.second->setListener(nullptr)) { + if (edp_readers.second && !edp_readers.second->setListener(nullptr)) { RMW_SET_ERROR_MSG("failed to unset EDPReader listener"); result_ret = RMW_RET_ERROR; } From 209cf6355304096c72e0a9856f311f35d71fdff5 Mon Sep 17 00:00:00 2001 From: Ethan Gao Date: Mon, 18 Dec 2017 18:39:14 +0800 Subject: [PATCH 5/5] check either edp_reader Signed-off-by: Ethan Gao --- rmw_fastrtps_cpp/src/rmw_node.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/rmw_fastrtps_cpp/src/rmw_node.cpp b/rmw_fastrtps_cpp/src/rmw_node.cpp index c5f9cead1..86306be10 100644 --- a/rmw_fastrtps_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_cpp/src/rmw_node.cpp @@ -127,13 +127,18 @@ create_node( node_impl->secondaryPubListener = tnat_2; edp_readers = participant->getEDPReaders(); - if (edp_readers.first && edp_readers.second) { - if (!(edp_readers.first->setListener(tnat_1) & edp_readers.second->setListener(tnat_2))) { - RMW_SET_ERROR_MSG("Failed to attach ROS related logic to the Participant"); - goto fail; - } - } else { - RMW_SET_ERROR_MSG("Failed to get valid reader for node subscriber and publisher"); + if (!edp_readers.first) { + RMW_SET_ERROR_MSG("edp_readers.first is null"); + goto fail; + } + + if (!edp_readers.second) { + RMW_SET_ERROR_MSG("edp_readers.second is null"); + goto fail; + } + + if (!(edp_readers.first->setListener(tnat_1) & edp_readers.second->setListener(tnat_2))) { + RMW_SET_ERROR_MSG("Failed to attach ROS related logic to the Participant"); goto fail; }