From 249ed388005576c76151087d59f1c09fc8f3b5f8 Mon Sep 17 00:00:00 2001 From: MyungJoo Ham Date: Mon, 7 Sep 2026 18:00:56 +0900 Subject: [PATCH 1/4] [Service] fix use-after-free on training offloading destroy _ml_service_training_offloading_destroy() released node_table - and with it every ml_service_node_info_s - before touching the pipeline, and it never stopped the pipeline at all. Every output node registers _ml_service_pipeline_sink_cb() with its node_info as user_data, and that callback dereferences node_info->mls and node_info->name. It also hands node_info->name to the application event data without copying it. So an application that calls ml_service_destroy() on a receiver still in PLAYING - which the API allows, ml_service_stop() is not mandatory - lets a buffer reaching the sink run the callback on a freed node_info. Release the pipeline first, mirroring _ml_service_extension_destroy(): ml_pipeline_stop() takes the pipeline out of PLAYING and ml_pipeline_destroy() brings it to NULL, which joins the streaming threads, so no callback can be in flight by the time the node info goes away. Stopping first also narrows the window in which ml_pipeline_destroy() tears down its named nodes while the pipeline is still running. Addresses item H3 of #690. Co-Authored-By: Claude Opus 5 Signed-off-by: MyungJoo Ham --- c/src/ml-api-service-training-offloading.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/c/src/ml-api-service-training-offloading.c b/c/src/ml-api-service-training-offloading.c index c8a8c1bb..f2b0075f 100644 --- a/c/src/ml-api-service-training-offloading.c +++ b/c/src/ml-api-service-training-offloading.c @@ -911,12 +911,10 @@ _ml_service_training_offloading_destroy (ml_service_s * mls) training_s->transfer_data_table = NULL; } - if (training_s->node_table) { - g_hash_table_destroy (training_s->node_table); - training_s->node_table = NULL; - } - + /* Stop the pipeline before releasing the node info the sink callback uses. */ if (training_s->pipeline_h) { + ml_pipeline_stop (training_s->pipeline_h); + ret = ml_pipeline_destroy (training_s->pipeline_h); if (ret != ML_ERROR_NONE) { _ml_error_report ("Failed to destroy ml pipeline, clear handle anyway."); @@ -925,6 +923,11 @@ _ml_service_training_offloading_destroy (ml_service_s * mls) training_s->pipeline_h = NULL; } + if (training_s->node_table) { + g_hash_table_destroy (training_s->node_table); + training_s->node_table = NULL; + } + g_clear_pointer (&training_s->path, g_free); g_clear_pointer (&training_s->trained_model_path, g_free); g_clear_pointer (&training_s->receiver_pipe_json_str, g_free); From ded70e8833b471e1a154a7ce8fce601b1eee1356 Mon Sep 17 00:00:00 2001 From: MyungJoo Ham Date: Mon, 7 Sep 2026 18:01:05 +0900 Subject: [PATCH 2/4] [Test] cover destroying a running training offloading service The existing training offloading test always stops the service before destroying it, so the teardown order was never exercised. destroyWhileRunning_p drives a receiver up to PLAYING and destroys it without stopping. The pipeline it uses is injected the way the remote sender would send it, but is self-contained (videotestsrc into tensor_sink): the teardown order is under test, not the training framework. The sink callback holds the streaming thread for 300 ms and then reads the node name back from the event data. That name is the node info's own string, passed on without a copy, so the read lands after the teardown has freed the node table if the pipeline is released too late. destroyAfterStop_p keeps the documented stop-then-destroy order working now that destroy stops the pipeline itself, and destroyInvalidParam2_n covers the guard that rejects a service which is not in training mode. Co-Authored-By: Claude Opus 5 Signed-off-by: MyungJoo Ham --- ...ittest_capi_service_training_offloading.cc | 175 ++++++++++++++++++ 1 file changed, 175 insertions(+) diff --git a/tests/capi/unittest_capi_service_training_offloading.cc b/tests/capi/unittest_capi_service_training_offloading.cc index db6968cc..2df876a7 100644 --- a/tests/capi/unittest_capi_service_training_offloading.cc +++ b/tests/capi/unittest_capi_service_training_offloading.cc @@ -8,10 +8,12 @@ */ #include +#include #include #include #include #include +#include #include "ml-api-service-offloading.h" #include "ml-api-service-training-offloading.h" @@ -332,6 +334,137 @@ TEST_F (MLServiceTrainingOffloading, create_p) EXPECT_EQ (g_remove (receiver_config), 0); } +/** + * @brief Pipeline the receiver would normally get from the remote sender. + * It is self-contained on purpose: the teardown order, not the training + * framework, is under test here. + */ +static const gchar *receiver_pipe_json + = R"JSON({"pipeline":{"description":"videotestsrc is-live=true ! videoconvert ! video/x-raw,format=RGB,width=16,height=16,framerate=30/1 ! tensor_converter ! tensor_sink name=training_result async=false","output_node":[{"name":"training_result"}]}})JSON"; + +/** + * @brief Time the sink callback stays in the pipeline, in microseconds. + */ +#define SINK_CB_HOLD_TIME (300000) + +/** + * @brief Callback holding the streaming thread while the service is destroyed. + * + * The 'name' of the event data is the name of the node info owned by the + * training offloading handle, handed over without a copy. Holding the callback + * makes the teardown overlap with it, so reading the name afterwards fails if + * the node info was released while the pipeline was still running. + */ +static void +_hold_new_data_cb (ml_service_event_e event, ml_information_h event_data, void *user_data) +{ + gint *received = (gint *) user_data; + char *node_name = NULL; + + if (event != ML_SERVICE_EVENT_NEW_DATA) + return; + + g_atomic_int_inc (received); + g_usleep (SINK_CB_HOLD_TIME); + + EXPECT_EQ (ml_information_get (event_data, "name", (void **) &node_name), ML_ERROR_NONE); + EXPECT_STREQ (node_name, "training_result"); +} + +/** + * @brief Bring a receiver service up to the point where its pipeline is + * playing and the sink callback is firing. + */ +static void +_start_receiver_pipeline (ml_service_h receiver_h, const gchar *path, gint *received) +{ + ml_service_s *mls = (ml_service_s *) receiver_h; + nns_edge_data_h data_h = NULL; + gint loop; + int status; + + status = _ml_service_training_offloading_set_path (mls, path); + ASSERT_EQ (status, ML_ERROR_NONE); + + status = ml_service_set_event_cb (receiver_h, _hold_new_data_cb, received); + ASSERT_EQ (status, ML_ERROR_NONE); + + ASSERT_EQ (nns_edge_data_create (&data_h), NNS_EDGE_ERROR_NONE); + status = _ml_service_training_offloading_process_received_data (mls, data_h, + path, receiver_pipe_json, ML_SERVICE_OFFLOADING_TYPE_PIPELINE_RAW); + nns_edge_data_destroy (data_h); + ASSERT_EQ (status, ML_ERROR_NONE); + + status = _ml_service_training_offloading_start (mls); + ASSERT_EQ (status, ML_ERROR_NONE); + + for (loop = 0; loop < 500 && g_atomic_int_get (received) == 0; loop++) + g_usleep (10000); + + ASSERT_GT (g_atomic_int_get (received), 0); +} + +/** + * @brief Destroying a running service must not release the node info the sink + * callback is still using. + */ +TEST_F (MLServiceTrainingOffloading, destroyWhileRunning_p) +{ + int status; + gint received = 0; + ml_service_h receiver_h = NULL; + g_autofree gchar *path = g_dir_make_tmp ("ml-training-offloading-XXXXXX", NULL); + + ASSERT_NE (nullptr, path); + + guint avail_port = get_available_port (); + g_autofree gchar *receiver_config + = prepare_test_config ("training_offloading_receiver.conf", avail_port); + + status = ml_service_new (receiver_config, &receiver_h); + ASSERT_EQ (status, ML_ERROR_NONE); + + _start_receiver_pipeline (receiver_h, path, &received); + + /* Destroy without stopping first. */ + status = ml_service_destroy (receiver_h); + EXPECT_EQ (ML_ERROR_NONE, status); + + EXPECT_EQ (g_remove (receiver_config), 0); + EXPECT_EQ (g_rmdir (path), 0); +} + +/** + * @brief Stopping the service before destroying it keeps working. + */ +TEST_F (MLServiceTrainingOffloading, destroyAfterStop_p) +{ + int status; + gint received = 0; + ml_service_h receiver_h = NULL; + g_autofree gchar *path = g_dir_make_tmp ("ml-training-offloading-XXXXXX", NULL); + + ASSERT_NE (nullptr, path); + + guint avail_port = get_available_port (); + g_autofree gchar *receiver_config + = prepare_test_config ("training_offloading_receiver.conf", avail_port); + + status = ml_service_new (receiver_config, &receiver_h); + ASSERT_EQ (status, ML_ERROR_NONE); + + _start_receiver_pipeline (receiver_h, path, &received); + + status = ml_service_stop (receiver_h); + EXPECT_EQ (ML_ERROR_NONE, status); + + status = ml_service_destroy (receiver_h); + EXPECT_EQ (ML_ERROR_NONE, status); + + EXPECT_EQ (g_remove (receiver_config), 0); + EXPECT_EQ (g_rmdir (path), 0); +} + /** * @brief Test _ml_service_training_offloading_destroy. */ @@ -343,6 +476,48 @@ TEST_F (MLServiceTrainingOffloading, destroyInvalidParam1_n) EXPECT_EQ (ML_ERROR_INVALID_PARAMETER, status); } +/** + * @brief Test _ml_service_training_offloading_destroy with a service that is + * not in training mode. + */ +TEST_F (MLServiceTrainingOffloading, destroyInvalidParam2_n) +{ + int status; + ml_service_s *mls; + + g_autoptr (JsonParser) parser = NULL; + g_autofree gchar *json_string = NULL; + JsonNode *root; + JsonObject *object; + + guint avail_port = get_available_port (); + g_autofree gchar *receiver_config + = prepare_test_config ("service_offloading_receiver.conf", avail_port); + + ASSERT_TRUE (g_file_get_contents (receiver_config, &json_string, NULL, NULL)); + parser = json_parser_new (); + ASSERT_TRUE (json_parser_load_from_data (parser, json_string, -1, NULL)); + root = json_parser_get_root (parser); + ASSERT_NE (nullptr, root); + object = json_node_get_object (root); + ASSERT_NE (nullptr, object); + + mls = _ml_service_create_internal (ML_SERVICE_TYPE_OFFLOADING); + ASSERT_NE (nullptr, mls); + + /* The configuration has no 'training' member, so the mode stays NONE. */ + status = _ml_service_offloading_create (mls, object); + EXPECT_EQ (ML_ERROR_NONE, status); + + status = _ml_service_training_offloading_destroy (mls); + EXPECT_EQ (ML_ERROR_INVALID_PARAMETER, status); + + status = _ml_service_offloading_release_internal (mls); + EXPECT_EQ (ML_ERROR_NONE, status); + + EXPECT_EQ (g_remove (receiver_config), 0); +} + /** * @brief Test _ml_service_training_offloading_set_path. */ From 063b4b1e674a560a9dad00cb1e0680f9f1fe3908 Mon Sep 17 00:00:00 2001 From: MyungJoo Ham Date: Mon, 7 Sep 2026 18:13:19 +0900 Subject: [PATCH 3/4] [Test] make the training offloading teardown test deterministic Review feedback on the regression test. The sink callback and the teardown only overlapped because the callback parked for longer than the main thread took to notice it. Replace the poll with a condition variable the callback signals on entry, so the destroy always starts while the callback is held. The check that distinguishes the two teardown orders reads a string the node table has already freed in the broken order, so what it reads back is up to the allocator. Enable M_PERTURB for the duration of the destroy call, and clear it right after to keep it away from the rest of the suite. This is a second net rather than the mechanism: glibc returns from tcache_put() before free_perturb() runs, and the node name is tcache sized, so for that chunk the clobber that the assertion actually sees is tcache_put() writing its link fields over the first 16 bytes. Both are allocator behaviour, not a property of the code under test, so a sanitizer build is still the only airtight way to catch this class of defect. The comment in the test says as much. Release the ml-service handle in destroyInvalidParam2_n with _ml_service_destroy_internal(), which also drops the handle itself and its ml-option, instead of only releasing the offloading private data. Co-Authored-By: Claude Opus 5 Signed-off-by: MyungJoo Ham --- ...ittest_capi_service_training_offloading.cc | 83 +++++++++++++++---- 1 file changed, 68 insertions(+), 15 deletions(-) diff --git a/tests/capi/unittest_capi_service_training_offloading.cc b/tests/capi/unittest_capi_service_training_offloading.cc index 2df876a7..2d2aa2dd 100644 --- a/tests/capi/unittest_capi_service_training_offloading.cc +++ b/tests/capi/unittest_capi_service_training_offloading.cc @@ -9,6 +9,9 @@ #include #include +#ifdef __GLIBC__ +#include +#endif #include #include #include @@ -347,6 +350,16 @@ static const gchar *receiver_pipe_json */ #define SINK_CB_HOLD_TIME (300000) +/** + * @brief Handshake between the sink callback and the thread tearing the + * service down. + */ +typedef struct { + GMutex lock; + GCond cond; + gboolean entered; +} sink_hold_s; + /** * @brief Callback holding the streaming thread while the service is destroyed. * @@ -358,13 +371,17 @@ static const gchar *receiver_pipe_json static void _hold_new_data_cb (ml_service_event_e event, ml_information_h event_data, void *user_data) { - gint *received = (gint *) user_data; + sink_hold_s *hold = (sink_hold_s *) user_data; char *node_name = NULL; if (event != ML_SERVICE_EVENT_NEW_DATA) return; - g_atomic_int_inc (received); + g_mutex_lock (&hold->lock); + hold->entered = TRUE; + g_cond_broadcast (&hold->cond); + g_mutex_unlock (&hold->lock); + g_usleep (SINK_CB_HOLD_TIME); EXPECT_EQ (ml_information_get (event_data, "name", (void **) &node_name), ML_ERROR_NONE); @@ -372,21 +389,22 @@ _hold_new_data_cb (ml_service_event_e event, ml_information_h event_data, void * } /** - * @brief Bring a receiver service up to the point where its pipeline is - * playing and the sink callback is firing. + * @brief Bring a receiver service up to the point where the sink callback has + * entered and is holding the streaming thread. */ static void -_start_receiver_pipeline (ml_service_h receiver_h, const gchar *path, gint *received) +_start_receiver_pipeline (ml_service_h receiver_h, const gchar *path, sink_hold_s *hold) { ml_service_s *mls = (ml_service_s *) receiver_h; nns_edge_data_h data_h = NULL; - gint loop; + gint64 deadline; + gboolean entered; int status; status = _ml_service_training_offloading_set_path (mls, path); ASSERT_EQ (status, ML_ERROR_NONE); - status = ml_service_set_event_cb (receiver_h, _hold_new_data_cb, received); + status = ml_service_set_event_cb (receiver_h, _hold_new_data_cb, hold); ASSERT_EQ (status, ML_ERROR_NONE); ASSERT_EQ (nns_edge_data_create (&data_h), NNS_EDGE_ERROR_NONE); @@ -398,10 +416,17 @@ _start_receiver_pipeline (ml_service_h receiver_h, const gchar *path, gint *rece status = _ml_service_training_offloading_start (mls); ASSERT_EQ (status, ML_ERROR_NONE); - for (loop = 0; loop < 500 && g_atomic_int_get (received) == 0; loop++) - g_usleep (10000); + deadline = g_get_monotonic_time () + 10 * G_TIME_SPAN_SECOND; - ASSERT_GT (g_atomic_int_get (received), 0); + g_mutex_lock (&hold->lock); + while (!hold->entered) { + if (!g_cond_wait_until (&hold->cond, &hold->lock, deadline)) + break; + } + entered = hold->entered; + g_mutex_unlock (&hold->lock); + + ASSERT_TRUE (entered); } /** @@ -411,7 +436,7 @@ _start_receiver_pipeline (ml_service_h receiver_h, const gchar *path, gint *rece TEST_F (MLServiceTrainingOffloading, destroyWhileRunning_p) { int status; - gint received = 0; + sink_hold_s hold = {}; ml_service_h receiver_h = NULL; g_autofree gchar *path = g_dir_make_tmp ("ml-training-offloading-XXXXXX", NULL); @@ -421,15 +446,37 @@ TEST_F (MLServiceTrainingOffloading, destroyWhileRunning_p) g_autofree gchar *receiver_config = prepare_test_config ("training_offloading_receiver.conf", avail_port); + g_mutex_init (&hold.lock); + g_cond_init (&hold.cond); + status = ml_service_new (receiver_config, &receiver_h); ASSERT_EQ (status, ML_ERROR_NONE); - _start_receiver_pipeline (receiver_h, path, &received); + _start_receiver_pipeline (receiver_h, path, &hold); + +#ifdef __GLIBC__ + /** + * Scrub memory as it is released, so that a node name read after the node + * table is gone is caught rather than read back intact. glibc skips this for + * a chunk that fits the tcache, which the node name normally does; there the + * detection instead comes from tcache_put() writing its own link fields over + * the first 16 bytes of the chunk. Neither is a property of the code under + * test, so a sanitizer build remains the only airtight net for this. + */ + mallopt (M_PERTURB, 0xAA); +#endif /* Destroy without stopping first. */ status = ml_service_destroy (receiver_h); EXPECT_EQ (ML_ERROR_NONE, status); +#ifdef __GLIBC__ + mallopt (M_PERTURB, 0); +#endif + + g_mutex_clear (&hold.lock); + g_cond_clear (&hold.cond); + EXPECT_EQ (g_remove (receiver_config), 0); EXPECT_EQ (g_rmdir (path), 0); } @@ -440,7 +487,7 @@ TEST_F (MLServiceTrainingOffloading, destroyWhileRunning_p) TEST_F (MLServiceTrainingOffloading, destroyAfterStop_p) { int status; - gint received = 0; + sink_hold_s hold = {}; ml_service_h receiver_h = NULL; g_autofree gchar *path = g_dir_make_tmp ("ml-training-offloading-XXXXXX", NULL); @@ -450,10 +497,13 @@ TEST_F (MLServiceTrainingOffloading, destroyAfterStop_p) g_autofree gchar *receiver_config = prepare_test_config ("training_offloading_receiver.conf", avail_port); + g_mutex_init (&hold.lock); + g_cond_init (&hold.cond); + status = ml_service_new (receiver_config, &receiver_h); ASSERT_EQ (status, ML_ERROR_NONE); - _start_receiver_pipeline (receiver_h, path, &received); + _start_receiver_pipeline (receiver_h, path, &hold); status = ml_service_stop (receiver_h); EXPECT_EQ (ML_ERROR_NONE, status); @@ -461,6 +511,9 @@ TEST_F (MLServiceTrainingOffloading, destroyAfterStop_p) status = ml_service_destroy (receiver_h); EXPECT_EQ (ML_ERROR_NONE, status); + g_mutex_clear (&hold.lock); + g_cond_clear (&hold.cond); + EXPECT_EQ (g_remove (receiver_config), 0); EXPECT_EQ (g_rmdir (path), 0); } @@ -512,7 +565,7 @@ TEST_F (MLServiceTrainingOffloading, destroyInvalidParam2_n) status = _ml_service_training_offloading_destroy (mls); EXPECT_EQ (ML_ERROR_INVALID_PARAMETER, status); - status = _ml_service_offloading_release_internal (mls); + status = _ml_service_destroy_internal (mls); EXPECT_EQ (ML_ERROR_NONE, status); EXPECT_EQ (g_remove (receiver_config), 0); From fb80b6d5b242ad5df93bbd3392041083ea8170f9 Mon Sep 17 00:00:00 2001 From: MyungJoo Ham Date: Mon, 7 Sep 2026 18:13:20 +0900 Subject: [PATCH 4/4] [Service] report a failing pipeline stop in training offloading destroy ml_pipeline_stop() can fail on a state change failure, and on Tizen it can also be refused by the feature check. Either way the pipeline is left for ml_pipeline_destroy() to pause on its own, which is the racy path the stop was added to avoid, so say so in the log the way the destroy call below already does. Co-Authored-By: Claude Opus 5 Signed-off-by: MyungJoo Ham --- c/src/ml-api-service-training-offloading.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/c/src/ml-api-service-training-offloading.c b/c/src/ml-api-service-training-offloading.c index f2b0075f..ec856941 100644 --- a/c/src/ml-api-service-training-offloading.c +++ b/c/src/ml-api-service-training-offloading.c @@ -913,7 +913,9 @@ _ml_service_training_offloading_destroy (ml_service_s * mls) /* Stop the pipeline before releasing the node info the sink callback uses. */ if (training_s->pipeline_h) { - ml_pipeline_stop (training_s->pipeline_h); + if (ml_pipeline_stop (training_s->pipeline_h) != ML_ERROR_NONE) { + _ml_error_report ("Failed to stop ml pipeline, destroy it anyway."); + } ret = ml_pipeline_destroy (training_s->pipeline_h); if (ret != ML_ERROR_NONE) {