You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by "yxscc (via GitHub)" <gi...@apache.org> on 2025/12/29 06:45:10 UTC

[I] [Bug] Celix ServiceTracker returns dangling pointers / UAF on concurrent unregister (deprecated API doesn’t retain useCount) (celix)

yxscc opened a new issue, #811:
URL: https://github.com/apache/celix/issues/811

   ### Summary
   `serviceTracker_getService` (deprecated) returns the service pointer without retaining `tracked->useCount` or waiting for ongoing unregister. If another thread unregisters the same service, `serviceTracker_untrackTracked` waits for useCount to drop to 1, then calls `bundleContext_ungetService` and frees `tracked`. Any caller still using the old pointer hits freed memory → UAF.
   
   ### Component
   - File: `libs/framework/src/service_tracker.c`
   - API: `serviceTracker_getService` (deprecated but still callable)
   - Scenario: concurrent “get service” vs “unregister service”
   
   ### Root Cause
   1. `serviceTracker_getService` only locks the list and returns `tracked->service`; it does not bump useCount or keep the entry alive.  
   2. `serviceTracker_untrackTracked` waits for useCount, then ungets and frees the tracked entry; previously returned raw pointers become dangling.  
   3. The function is marked “TODO deprecated warning -> not locked” but is neither disabled nor concurrency-safe, so multithreaded callers (common in async callbacks) hit UAF.
   
   Key code excerpts:
   
   ```20:78:libs/framework/src/service_tracker.c
   // returns raw pointer, no useCount retain
   void *serviceTracker_getService(service_tracker_t* tracker) {
       //TODO deprecated warning -> not locked
       void *service = NULL;
       celixThreadMutex_lock(&tracker->state.mutex);
       ... // find tracked->service
       celixThreadMutex_unlock(&tracker->state.mutex);
       return service;
   }
   ```
   
   ```502:575:libs/framework/src/service_tracker.c
   // unregister path eventually frees tracked
   static void serviceTracker_untrackTracked(...) {
       serviceTracker_invokeRemovingService(tracker, tracked);
       ...
       celixThreadMutex_lock(&tracked->mutex);
       while (tracked->useCount > 1) {
           celixThreadCondition_wait(&tracked->useCond, &tracked->mutex);
       }
       celixThreadMutex_unlock(&tracked->mutex);
   
       bundleContext_ungetService(tracker->context, tracked->reference, &ungetSuccess);
       bundleContext_ungetServiceReference(tracker->context, tracked->reference);
       assert(tracked->useCount == 1);
       ... // destroy mutex/cond & free(tracked)
   }
   ```
   
   ### Reproduction (self-contained, suitable for Issue review)
   
   1) Minimal gtest (drop into any test target):
   ```cpp
   TEST(ServiceTrackerUafRepro, DeprecatedGetServiceUaf) {
       struct dummy { uint64_t magic; };
       celix_properties_t* props = celix_properties_create();
       celix_properties_setBool(props, CELIX_FRAMEWORK_CLEAN_CACHE_DIR_ON_CREATE, true);
       celix_properties_set(props, CELIX_FRAMEWORK_CACHE_DIR, ".cacheServiceTrackerUaf");
       celix_framework_t* fw = celix_frameworkFactory_createFramework(props);
       auto* ctx = celix_framework_getFrameworkContext(fw);
   
       auto* svc = static_cast<dummy*>(malloc(sizeof(dummy)));
       svc->magic = 0xBEEFBEEF;
       long svcId = celix_bundleContext_registerService(ctx, svc, "dummy_uaf", nullptr);
   
       int handle = 0; // non-null handle required
       service_tracker_customizer_pt cust = nullptr;
       ASSERT_EQ(CELIX_SUCCESS, serviceTrackerCustomizer_create(&handle, nullptr, nullptr, nullptr, nullptr, &cust));
       service_tracker_t* tracker = nullptr;
       ASSERT_EQ(CELIX_SUCCESS, serviceTracker_create(ctx, "dummy_uaf", cust, &tracker));
       ASSERT_EQ(CELIX_SUCCESS, serviceTracker_open(tracker));
   
       std::atomic<bool> stop{false};
       std::thread tGet([&] {
           while (!stop.load()) {
               auto* s = static_cast<dummy*>(serviceTracker_getService(tracker));
               if (s) { s->magic = 0xCAFEBABECAFED00DULL; }
           }
       });
       std::thread tUnreg([&] {
           for (int i = 0; i < 2000 && !stop.load(); ++i) {
               celix_bundleContext_unregisterService(ctx, svcId);
               free(svc); // provider frees immediately
               svc = static_cast<dummy*>(malloc(sizeof(dummy)));
               svc->magic = 0xDEADBEEF + i;
               svcId = celix_bundleContext_registerService(ctx, svc, "dummy_uaf", nullptr);
           }
           stop.store(true);
       });
       tUnreg.join();
       stop.store(true);
       tGet.join();
   
       celix_bundleContext_unregisterService(ctx, svcId);
       free(svc);
       serviceTracker_close(tracker);
       serviceTracker_destroy(tracker);
       serviceTrackerCustomizer_destroy(cust);
       celix_frameworkFactory_destroyFramework(fw);
       // Expected: crash / ASan UAF because getService returns a freed object.
   }
   ```
   
   2) Add the test file to `libs/framework/gtest` and list it in `CELIX_FRAMEWORK_TEST_SOURCES` in `libs/framework/gtest/CMakeLists.txt`.
   
   3) Build & run (Debug/ASan recommended):
   ```
   cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug -DENABLE_TESTING=ON -DBUILD_TESTING=ON
   cmake --build build --target test_framework -j4
   cd build && ./libs/framework/gtest/test_frameworkd --gtest_filter=*ServiceTrackerUaf*
   ```
   
   4) Expected result: runtime crash like `free(): invalid size` (exit 134) or ASan reports use-after-free; this shows `serviceTracker_getService` returns a dangling pointer when unregister races.
   
   ### Impact
   - Callers may get freed service pointers under concurrency, leading to crashes or wrong behavior.  
   - Misuse risk is high because the deprecated API is still callable and older code may rely on it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Bug] Celix ServiceTracker returns dangling pointers / UAF on concurrent unregister (deprecated API doesn’t retain useCount) (celix)

Posted by "yxscc (via GitHub)" <gi...@apache.org>.
yxscc commented on issue #811:
URL: https://github.com/apache/celix/issues/811#issuecomment-3697998845

   Additional finding: the same race/UAF exists in deprecated `serviceTracker_getServices`. The returned array holds raw service pointers without retaining useCount; during concurrent unregister, `serviceTracker_untrackTracked` ungets and frees the tracked entry, leaving array entries dangling.
   
   Repro: two threads — one repeatedly calls `serviceTracker_getServices` and touches entries; the other repeatedly unregisters/re-registers the same service (freeing the old instance). In Debug/ASan, the gtest `ServiceTrackerUafGetServicesTestSuite.DeprecatedGetServicesReturnsFreedPointers` crashes with `free(): invalid size` (exit 134), showing that `getServices` can return freed objects under unregister races.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Bug] Celix ServiceTracker returns dangling pointers / UAF on concurrent unregister (deprecated API doesn’t retain useCount) (celix)

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on issue #811:
URL: https://github.com/apache/celix/issues/811#issuecomment-3698383478

   > [@PengZheng](https://github.com/PengZheng) would it be an idea to link the applicable tickets to the correct release milestone (I assume `3.0.0`) for users to know upfront what will be fixed in the next release?
   
   Good idea. IIRC, installation of these headers will be removed in 3.0.0 as mentioned in https://github.com/apache/celix/issues/509
   Moreover, these functions are broken for a very long time, it would be safe to assume that there are no downstream users.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Bug] Celix ServiceTracker returns dangling pointers / UAF on concurrent unregister (deprecated API doesn’t retain useCount) (celix)

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on issue #811:
URL: https://github.com/apache/celix/issues/811#issuecomment-3695624634

   Yes, `serviceTracker_getService` is buggy, and will be removed in the coming release.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Bug] Celix ServiceTracker returns dangling pointers / UAF on concurrent unregister (deprecated API doesn’t retain useCount) (celix)

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng closed issue #811: [Bug] Celix ServiceTracker returns dangling pointers / UAF on concurrent unregister (deprecated API doesn’t retain useCount)
URL: https://github.com/apache/celix/issues/811


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Bug] Celix ServiceTracker returns dangling pointers / UAF on concurrent unregister (deprecated API doesn’t retain useCount) (celix)

Posted by "rlenferink (via GitHub)" <gi...@apache.org>.
rlenferink commented on issue #811:
URL: https://github.com/apache/celix/issues/811#issuecomment-3696337851

   @PengZheng would it be an idea to link the applicable tickets to the correct release milestone (I assume `3.0.0`) for users to know upfront what will be fixed in the next release?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org