diff --git a/test/libscap/test_suites/engines/savefile/convert_event_test.h b/test/libscap/test_suites/engines/savefile/convert_event_test.h index 99cc22b014..5b2462a49a 100644 --- a/test/libscap/test_suites/engines/savefile/convert_event_test.h +++ b/test/libscap/test_suites/engines/savefile/convert_event_test.h @@ -27,10 +27,13 @@ class convert_event_test : public testing::Test { static constexpr uint16_t safe_margin = 100; protected: - virtual void TearDown() { - // At every iteration we want to clear the storage in the converter - scap_clear_converter_storage(); + virtual void SetUp() { + m_converter_buf = scap_convert_alloc_buffer(); + ASSERT_NE(m_converter_buf, nullptr); } + + virtual void TearDown() { scap_convert_free_buffer(m_converter_buf); } + safe_scap_evt_t create_safe_scap_event(uint64_t ts, uint64_t tid, ppm_event_code event_type, @@ -54,7 +57,8 @@ class convert_event_test : public testing::Test { // We assume it's okay to create a new event with the same size as the expected event auto storage = new_safe_scap_evt((scap_evt *)calloc(1, expected_evt->len)); // First we check the conversion result matches the expected result - ASSERT_EQ(scap_convert_event(storage.get(), evt_to_convert.get(), error), expected_res) + ASSERT_EQ(scap_convert_event(m_converter_buf, storage.get(), evt_to_convert.get(), error), + expected_res) << "Different conversion results: " << error; if(!scap_compare_events(storage.get(), expected_evt.get(), error)) { printf("\nExpected event:\n"); @@ -69,7 +73,8 @@ class convert_event_test : public testing::Test { // We assume it's okay to create a new event with the same size as the expected event auto storage = new_safe_scap_evt((scap_evt *)calloc(1, evt_to_convert->len)); // First we check the conversion result matches the expected result - ASSERT_EQ(scap_convert_event(storage.get(), evt_to_convert.get(), error), CONVERSION_ERROR) + ASSERT_EQ(scap_convert_event(m_converter_buf, storage.get(), evt_to_convert.get(), error), + CONVERSION_ERROR) << "The conversion is not failed: " << error; } void assert_single_conversion_skip(safe_scap_evt_t evt_to_convert) { @@ -77,7 +82,8 @@ class convert_event_test : public testing::Test { // We assume it's okay to create a new event with the same size as the expected event auto storage = new_safe_scap_evt((scap_evt *)calloc(1, evt_to_convert->len)); // First we check the conversion result matches the expected result - ASSERT_EQ(scap_convert_event(storage.get(), evt_to_convert.get(), error), CONVERSION_SKIP) + ASSERT_EQ(scap_convert_event(m_converter_buf, storage.get(), evt_to_convert.get(), error), + CONVERSION_SKIP) << "The conversion is not skipped: " << error; } void assert_full_conversion(safe_scap_evt_t evt_to_convert, safe_scap_evt_t expected_evt) { @@ -99,7 +105,8 @@ class convert_event_test : public testing::Test { conv_num++) { // Copy the new event into the one to convert for the next conversion. memcpy(to_convert_evt.get(), new_evt.get(), new_evt->len); - conv_res = scap_convert_event((scap_evt *)new_evt.get(), + conv_res = scap_convert_event(m_converter_buf, + (scap_evt *)new_evt.get(), (scap_evt *)to_convert_evt.get(), error); } @@ -128,7 +135,7 @@ class convert_event_test : public testing::Test { void assert_event_storage_presence(safe_scap_evt_t expected_evt) { char error[SCAP_LASTERR_SIZE] = {'\0'}; int64_t tid = expected_evt.get()->tid; - auto event = scap_retrieve_evt_from_converter_storage(tid); + auto event = scap_retrieve_evt_from_converter_storage(m_converter_buf, tid); if(!event) { FAIL() << "Event with tid " << tid << " not found in the storage"; } @@ -136,4 +143,6 @@ class convert_event_test : public testing::Test { FAIL() << "Different events: " << error; } } + + struct scap_convert_buffer *m_converter_buf = nullptr; }; diff --git a/userspace/libscap/engine/savefile/converter/converter.cpp b/userspace/libscap/engine/savefile/converter/converter.cpp index d6eb509441..6b37517a4b 100644 --- a/userspace/libscap/engine/savefile/converter/converter.cpp +++ b/userspace/libscap/engine/savefile/converter/converter.cpp @@ -34,8 +34,9 @@ static inline safe_scap_evt_t safe_scap_evt(scap_evt *evt) { return safe_scap_evt_t{evt, free}; } -// use a shared pointer to store the events -static std::unordered_map evt_storage = {}; +struct scap_convert_buffer { + std::unordered_map evt_storage = {}; +}; static const char *get_event_name(ppm_event_code event_type) { const struct ppm_event_info *event_info = &g_event_info[event_type]; @@ -50,19 +51,21 @@ static char get_direction_char(ppm_event_code event_type) { } } -static void clear_evt(uint64_t tid) { +static void clear_evt(std::unordered_map &evt_storage, uint64_t tid) { if(evt_storage.find(tid) != evt_storage.end()) { evt_storage[tid].reset(); } } -static void store_evt(uint64_t tid, scap_evt *evt) { +static void store_evt(std::unordered_map &evt_storage, + uint64_t tid, + scap_evt *evt) { // if there was a previous event for this tid, we can overwrite the pointer because it means we // don't need it anymore. We need to keep the enter event until we retrieve it in the // corresponding exit event, but if the same thread is doing another enter event it means the // previous syscall is already completed. - clear_evt(tid); + clear_evt(evt_storage, tid); scap_evt *tmp_evt = (scap_evt *)malloc(evt->len); if(!tmp_evt) { @@ -72,7 +75,8 @@ static void store_evt(uint64_t tid, scap_evt *evt) { evt_storage[tid] = safe_scap_evt(tmp_evt); } -static scap_evt *retrieve_evt(uint64_t tid) { +static scap_evt *retrieve_evt(std::unordered_map &evt_storage, + uint64_t tid) { if(evt_storage.find(tid) != evt_storage.end()) { return evt_storage[tid].get(); } @@ -289,15 +293,19 @@ extern "C" bool is_conversion_needed(scap_evt *evt_to_convert) { return false; } -extern "C" scap_evt *scap_retrieve_evt_from_converter_storage(uint64_t tid) { - return retrieve_evt(tid); +extern "C" scap_evt *scap_retrieve_evt_from_converter_storage( + std::unordered_map &evt_storage, + uint64_t tid) { + return retrieve_evt(evt_storage, tid); } -extern "C" void scap_clear_converter_storage() { +extern "C" void scap_clear_converter_storage( + std::unordered_map &evt_storage) { evt_storage.clear(); } -static conversion_result convert_event(scap_evt *new_evt, +static conversion_result convert_event(std::unordered_map &evt_storage, + scap_evt *new_evt, scap_evt *evt_to_convert, const conversion_info &ci, char *error) { @@ -326,7 +334,7 @@ static conversion_result convert_event(scap_evt *new_evt, return CONVERSION_SKIP; case C_ACTION_STORE: - store_evt(evt_to_convert->tid, evt_to_convert); + store_evt(evt_storage, evt_to_convert->tid, evt_to_convert); return CONVERSION_SKIP; case C_ACTION_ADD_PARAMS: @@ -368,7 +376,7 @@ static conversion_result convert_event(scap_evt *new_evt, break; case C_INSTR_FROM_ENTER: - tmp_evt = retrieve_evt(evt_to_convert->tid); + tmp_evt = retrieve_evt(evt_storage, evt_to_convert->tid); if(!tmp_evt) { // It could be due to different reasons: // - we dropped the enter event in the capture @@ -430,7 +438,7 @@ static conversion_result convert_event(scap_evt *new_evt, if(PPME_IS_EXIT(evt_to_convert->type)) { // We can free the enter event for this thread because we don't need it anymore. - clear_evt(evt_to_convert->tid); + clear_evt(evt_storage, evt_to_convert->tid); } new_evt->len = params_offset; @@ -439,7 +447,12 @@ static conversion_result convert_event(scap_evt *new_evt, return is_conversion_needed(new_evt) ? CONVERSION_CONTINUE : CONVERSION_COMPLETED; } -extern "C" conversion_result scap_convert_event(scap_evt *new_evt, +extern "C" struct scap_convert_buffer *scap_convert_alloc_buffer() { + return new scap_convert_buffer(); +} + +extern "C" conversion_result scap_convert_event(struct scap_convert_buffer *buf, + scap_evt *new_evt, scap_evt *evt_to_convert, char *error) { // This should be checked by the caller but just double check here @@ -464,5 +477,13 @@ extern "C" conversion_result scap_convert_event(scap_evt *new_evt, } // If we reached this point we have for sure an entry in the conversion table. - return convert_event(new_evt, evt_to_convert, g_conversion_table.at(conv_key), error); + return convert_event(buf->evt_storage, + new_evt, + evt_to_convert, + g_conversion_table.at(conv_key), + error); +} + +extern "C" void scap_convert_free_buffer(struct scap_convert_buffer *buf) { + delete buf; } diff --git a/userspace/libscap/engine/savefile/converter/converter.h b/userspace/libscap/engine/savefile/converter/converter.h index 338486b103..1cdcf3f96e 100644 --- a/userspace/libscap/engine/savefile/converter/converter.h +++ b/userspace/libscap/engine/savefile/converter/converter.h @@ -29,13 +29,19 @@ typedef struct ppm_evt_hdr scap_evt; // 50 consecutive conversions on the same event should be more than enough #define MAX_CONVERSION_BOUNDARY 50 -conversion_result scap_convert_event(scap_evt* new_evt, scap_evt* evt_to_convert, char* error); +struct scap_convert_buffer; + +struct scap_convert_buffer* scap_convert_alloc_buffer(); +conversion_result scap_convert_event(struct scap_convert_buffer* buf, + scap_evt* new_evt, + scap_evt* evt_to_convert, + char* error); +void scap_convert_free_buffer(struct scap_convert_buffer* buf); bool is_conversion_needed(scap_evt* evt_to_convert); // Only for testing purposes -scap_evt* scap_retrieve_evt_from_converter_storage(uint64_t tid); -void scap_clear_converter_storage(); +scap_evt* scap_retrieve_evt_from_converter_storage(struct scap_convert_buffer* buf, uint64_t tid); #ifdef __cplusplus }; diff --git a/userspace/libscap/engine/savefile/savefile.h b/userspace/libscap/engine/savefile/savefile.h index f39f00dd5f..2a16f9003f 100644 --- a/userspace/libscap/engine/savefile/savefile.h +++ b/userspace/libscap/engine/savefile/savefile.h @@ -113,4 +113,5 @@ struct savefile_engine { // Used by the scap-file converter char* m_new_evt; char* m_to_convert_evt; + struct scap_convert_buffer* m_converter_buf; }; diff --git a/userspace/libscap/engine/savefile/scap_savefile.c b/userspace/libscap/engine/savefile/scap_savefile.c index e4b7da1d41..cc1f6a9fb8 100644 --- a/userspace/libscap/engine/savefile/scap_savefile.c +++ b/userspace/libscap/engine/savefile/scap_savefile.c @@ -2053,7 +2053,8 @@ static int32_t next(struct scap_engine_handle engine, conv_num++) { // Before each conversion we move the current event into the storage memcpy(handle->m_to_convert_evt, *pevent, (*pevent)->len); - conv_res = scap_convert_event((scap_evt *)handle->m_new_evt, + conv_res = scap_convert_event(handle->m_converter_buf, + (scap_evt *)handle->m_new_evt, (scap_evt *)handle->m_to_convert_evt, handle->m_lasterr); // At the end of the conversion in any case we swith to the new event pointer @@ -2239,6 +2240,14 @@ static int32_t init(struct scap *main_handle, struct scap_open_args *oargs) { } } + handle->m_converter_buf = scap_convert_alloc_buffer(); + if(!handle->m_converter_buf) { + snprintf(main_handle->m_lasterr, + SCAP_LASTERR_SIZE, + "error allocating the conversion buffer"); + return SCAP_FAILURE; + } + return SCAP_SUCCESS; } @@ -2268,6 +2277,11 @@ static int32_t scap_savefile_close(struct scap_engine_handle engine) { handle->m_to_convert_evt = NULL; } + if(handle->m_converter_buf) { + scap_convert_free_buffer(handle->m_converter_buf); + handle->m_converter_buf = NULL; + } + return SCAP_SUCCESS; }