From 538afcb7e06f7644c8c930de5c647f38f3cabb52 Mon Sep 17 00:00:00 2001
From: Jonathan Hanks <jonathan.hanks@ligo.org>
Date: Tue, 13 Oct 2020 12:22:51 -0700
Subject: [PATCH] More work on #186. Adding checks in shmem_receiver.hh for
cycle and segment sizes.
---
src/daqd/shmem_receiver.hh | 71 +++++++++++++++++++++----------
src/daqd/tests/test_shmem_recv.cc | 53 +++++++++++++++++++++--
2 files changed, 97 insertions(+), 27 deletions(-)
diff --git a/src/daqd/shmem_receiver.hh b/src/daqd/shmem_receiver.hh
index bbb1af586..f4fcb1349 100644
--- a/src/daqd/shmem_receiver.hh
+++ b/src/daqd/shmem_receiver.hh
@@ -10,42 +10,30 @@
#include <chrono>
#include <functional>
#include <thread>
+#include <sstream>
#include "daq_core.h"
#include "drv/shmem.h"
class ShMemReceiver
{
- volatile daq_multi_cycle_data_t* shmem_;
-
- daq_dc_data_t data_;
- unsigned int prev_cycle_;
- unsigned int prev_gps_;
- std::function< void( void ) > signal_stalled_;
-
- void
- wait( )
- {
- std::this_thread::sleep_for( std::chrono::milliseconds( 1 ) );
- }
-
public:
ShMemReceiver( ) = delete;
ShMemReceiver( ShMemReceiver& other ) = delete;
ShMemReceiver( const std::string& endpoint,
- size_t shmem_size,
+ std::size_t shmem_size,
std::function< void( void ) > signal = []( ) {} )
: shmem_( static_cast< volatile daq_multi_cycle_data_t* >(
shmem_open_segment( endpoint.c_str( ), shmem_size ) ) ),
- prev_cycle_( 0xffffffff ),
- prev_gps_( 0xffffffff ), signal_stalled_{ std::move( signal ) }
+ shmem_size_( shmem_size ), signal_stalled_{ std::move( signal ) }
{
}
- explicit ShMemReceiver( volatile daq_multi_cycle_data_t* shmem_area,
- std::function< void( void ) > signal = []( ) {} )
- : shmem_( shmem_area ), prev_cycle_( 0xffffffff ),
- prev_gps_( 0xffffffff ), signal_stalled_{ std::move( signal ) }
+ ShMemReceiver( volatile daq_multi_cycle_data_t* shmem_area,
+ std::size_t shmem_size,
+ std::function< void( void ) > signal = []( ) {} )
+ : shmem_( shmem_area ),
+ shmem_size_( shmem_size ), signal_stalled_{ std::move( signal ) }
{
}
@@ -84,7 +72,9 @@ public:
signal_stalled_( );
}
}
- cur_cycle = handle_cycle_jumps( cur_cycle );
+ unsigned int max_cycle = get_max_cycle( );
+ verify_cycle_state( cur_cycle, max_cycle );
+ cur_cycle = handle_cycle_jumps( cur_cycle, max_cycle );
unsigned int cycle_stride = shmem_->header.cycleDataSize;
// figure out offset to the right block
@@ -121,10 +111,31 @@ private:
.timeSec;
}
+ void
+ verify_cycle_state( unsigned int cur_cycle, unsigned int max_cycle )
+ {
+ if ( cur_cycle >= max_cycle || max_cycle > 64 )
+ {
+ throw std::runtime_error(
+ "Invalid shmem header or nonsensical values" );
+ }
+ auto cycleDataSize = shmem_->header.cycleDataSize;
+ if ( cycleDataSize * max_cycle +
+ sizeof( daq_multi_cycle_header_t ) >
+ shmem_size_ )
+ {
+ std::ostringstream os;
+ os << "Overflow condition exists, the cycleDataSize is wrong for the "
+ "shared memory size. cycleDataSize=" << cycleDataSize << " max_cycle="
+ << max_cycle << " header_size=" << sizeof( daq_multi_cycle_header_t) << " shmem_size=" << shmem_size_;
+ std::string msg = os.str();
+ throw std::runtime_error( msg );
+ }
+ }
+
int
- handle_cycle_jumps( unsigned int new_cycle )
+ handle_cycle_jumps( unsigned int new_cycle, unsigned int max_cycle )
{
- unsigned int max_cycle = get_max_cycle( );
unsigned int prev = prev_cycle_;
unsigned int prev_gps = prev_gps_;
@@ -149,6 +160,20 @@ private:
}
return new_cycle;
}
+
+ void
+ wait( )
+ {
+ std::this_thread::sleep_for( std::chrono::milliseconds( 3 ) );
+ }
+
+ volatile daq_multi_cycle_data_t* shmem_;
+ std::size_t shmem_size_;
+
+ daq_dc_data_t data_{};
+ unsigned int prev_cycle_{ 0xffffffff };
+ unsigned int prev_gps_{ 0xffffffff };
+ std::function< void( void ) > signal_stalled_;
};
#endif // DAQD_// SHMEM_RECEIVER_HH
diff --git a/src/daqd/tests/test_shmem_recv.cc b/src/daqd/tests/test_shmem_recv.cc
index 78cb5d441..84d8748a0 100644
--- a/src/daqd/tests/test_shmem_recv.cc
+++ b/src/daqd/tests/test_shmem_recv.cc
@@ -32,7 +32,7 @@ TEST_CASE(
}
ShMemReceiver recv(
- reinterpret_cast< volatile daq_multi_cycle_data_t* >( shmem.get( ) ) );
+ reinterpret_cast< volatile daq_multi_cycle_data_t* >( shmem.get( ) ), sizeof( daq_multi_cycle_data_t) );
recv.reset_previous_cycle( 4, 1000000000 );
auto data = recv.receive_data( );
REQUIRE( data->dataBlock[ 0 ] == 5 );
@@ -62,7 +62,7 @@ TEST_CASE( "ShMemReceiver should try to catch up if it sees a jump" )
}
ShMemReceiver recv(
- reinterpret_cast< volatile daq_multi_cycle_data_t* >( shmem.get( ) ) );
+ reinterpret_cast< volatile daq_multi_cycle_data_t* >( shmem.get( ) ), sizeof( daq_multi_cycle_data_t ) );
recv.reset_previous_cycle( 0, 1000000000 );
auto data = recv.receive_data( );
REQUIRE( data->dataBlock[ 0 ] == 1 );
@@ -94,7 +94,8 @@ TEST_CASE( "ShMemReceiver should try to catch up if it sees a jump, even when "
}
ShMemReceiver recv(
- reinterpret_cast< volatile daq_multi_cycle_data_t* >( shmem.get( ) ) );
+ reinterpret_cast< volatile daq_multi_cycle_data_t* >( shmem.get( ) ),
+ sizeof( daq_multi_cycle_data_t ) );
recv.reset_previous_cycle( 10, 1000000000 );
auto data = recv.receive_data( );
REQUIRE( data->dataBlock[ 0 ] == 11 );
@@ -124,8 +125,52 @@ TEST_CASE( "ShMemReceiver should try to catch up but will jump if needed" )
}
ShMemReceiver recv(
- reinterpret_cast< volatile daq_multi_cycle_data_t* >( shmem.get( ) ) );
+ reinterpret_cast< volatile daq_multi_cycle_data_t* >( shmem.get( ) ),
+ sizeof( daq_multi_cycle_data_t ) );
recv.reset_previous_cycle( 0, 1000000000 - 1 );
auto data = recv.receive_data( );
REQUIRE( data->dataBlock[ 0 ] == 5 );
+}
+
+TEST_CASE( "ShMemReceiver will throw an exception if max cycles is too big")
+{
+ auto shmem = raii::make_unique_ptr< daq_multi_cycle_data_t >( );
+ shmem->header.maxCycle = 65;
+ shmem->header.curCycle = 5;
+ shmem->header.cycleDataSize = sizeof( shmem->dataBlock ) / 16;
+ ShMemReceiver recv(
+ reinterpret_cast< volatile daq_multi_cycle_data_t* >( shmem.get( ) ),
+ sizeof( daq_multi_cycle_data_t ) );
+ recv.reset_previous_cycle( 0, 1000000000 - 1 );
+ REQUIRE_THROWS_AS(recv.receive_data(), std::runtime_error);
+}
+
+TEST_CASE( "ShMemReceiver will throw an exception if curCycle is too big")
+{
+ auto shmem = raii::make_unique_ptr< daq_multi_cycle_data_t >( );
+ shmem->header.maxCycle = 16;
+ shmem->header.curCycle = 15;
+ shmem->header.cycleDataSize = sizeof( shmem->dataBlock ) / 16;
+ ShMemReceiver recv(
+ reinterpret_cast< volatile daq_multi_cycle_data_t* >( shmem.get( ) ),
+ sizeof( daq_multi_cycle_data_t ) );
+ recv.reset_previous_cycle( 15, 1000000000 - 1 );
+ shmem->header.curCycle = 16;
+ REQUIRE_THROWS_AS(recv.receive_data(), std::runtime_error);
+}
+
+TEST_CASE( "ShMemReceiver will throw an exception if cycleDataSize*maxCycle is too big")
+{
+ auto shmem = raii::make_unique_ptr< daq_multi_cycle_data_t >( );
+ shmem->header.maxCycle = 16;
+ shmem->header.curCycle = 15;
+ shmem->header.cycleDataSize = sizeof( shmem->dataBlock ) / 16;
+ ShMemReceiver recv(
+ reinterpret_cast< volatile daq_multi_cycle_data_t* >( shmem.get( ) ),
+ sizeof( daq_multi_cycle_data_t ) );
+ recv.reset_previous_cycle( 15, 1000000000 - 1 );
+ shmem->header.curCycle = 0;
+ // this does not take away the header size, so it should lead to an overflow condition
+ shmem->header.cycleDataSize = sizeof(daq_multi_cycle_data_t)/16;
+ REQUIRE_THROWS_AS(recv.receive_data(), std::runtime_error);
}
\ No newline at end of file
--
GitLab