Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hotfix/fix use-after-free caused by cancelled service tracker. #596

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,7 @@ celix_status_t discoveryZeroconfWatcher_create(celix_bundle_context_t *ctx, celi
*watcherOut = watcher;
return CELIX_SUCCESS;
thread_err:
celix_bundleContext_stopTrackerAsync(ctx, watcher->epListenerTrkId, NULL, NULL);
celix_bundleContext_waitForAsyncStopTracker(ctx, watcher->epListenerTrkId);
celix_bundleContext_stopTracker(ctx, watcher->epListenerTrkId);
epl_tracker_err:
celix_longHashMap_destroy(watcher->epls);
celix_stringHashMap_destroy(watcher->watchedServices);
Expand Down
9 changes: 9 additions & 0 deletions libs/framework/include_deprecated/service_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,15 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_service_tracker_t* celix_serviceTracker_

);

/**
* Creates a closed service tracker.
* It is similar to serviceTracker_create.
*/
CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_service_tracker_t* celix_serviceTracker_createClosedWithOptions(
celix_bundle_context_t *ctx,
const celix_service_tracking_options_t *opts
);

/**
* Creates and starts (open) a service tracker.
* Note that is different from the serviceTracker_create function, because is also starts the service tracker
Expand Down
27 changes: 11 additions & 16 deletions libs/framework/src/bundle_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -1235,25 +1235,20 @@ static void celix_bundleContext_createTrackerOnEventLoop(void *data) {
assert(celix_framework_isCurrentThreadTheEventLoop(entry->ctx->framework));
celixThreadMutex_lock(&entry->ctx->mutex);
bool cancelled = entry->cancelled;
celixThreadMutex_unlock(&entry->ctx->mutex);
if (cancelled) {
fw_log(entry->ctx->framework->logger, CELIX_LOG_LEVEL_DEBUG, "Creating of service tracker was cancelled. trk id = %li, svc name tracked = %s", entry->trackerId, entry->opts.filter.serviceName);
celixThreadMutex_unlock(&entry->ctx->mutex);
return;
}
celix_service_tracker_t *tracker = celix_serviceTracker_createClosedWithOptions(entry->ctx, &entry->opts);
if (tracker == NULL) {
fw_log(entry->ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot create tracker for bnd %s (%li)", celix_bundle_getSymbolicName(entry->ctx->bundle), celix_bundle_getId(entry->ctx->bundle));
} else {
celix_service_tracker_t *tracker = celix_serviceTracker_createWithOptions(entry->ctx, &entry->opts);
if (tracker != NULL) {
celixThreadMutex_lock(&entry->ctx->mutex);
// double-check is necessary to eliminate first-check-then-do race condition
if(!entry->cancelled) {
entry->tracker = tracker;
}
celixThreadMutex_unlock(&entry->ctx->mutex);
if(entry->tracker == NULL) {
assert(entry->cancelled);
celix_serviceTracker_destroy(tracker);
}
} else {
fw_log(entry->ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot create tracker for bnd %s (%li)", celix_bundle_getSymbolicName(entry->ctx->bundle), celix_bundle_getId(entry->ctx->bundle));
}
entry->tracker = tracker;
}
celixThreadMutex_unlock(&entry->ctx->mutex);
if (tracker) {
serviceTracker_open(tracker);
}
}

Expand Down
45 changes: 31 additions & 14 deletions libs/framework/src/service_tracker.c
Original file line number Diff line number Diff line change
Expand Up @@ -632,36 +632,43 @@ celix_service_tracker_t* celix_serviceTracker_create(
return celix_serviceTracker_createWithOptions(ctx, &opts);
}

celix_service_tracker_t* celix_serviceTracker_createWithOptions(
bundle_context_t *ctx,
const celix_service_tracking_options_t *opts
) {
celix_service_tracker_t *tracker = NULL;
celix_service_tracker_t* celix_serviceTracker_createClosedWithOptions(celix_bundle_context_t* ctx,
const celix_service_tracking_options_t* opts) {
celix_service_tracker_t* tracker = NULL;
const char* serviceName = NULL;
char *filter = NULL;
char* filter = NULL;
assert(ctx != NULL);
assert(opts != NULL);
serviceName = opts->filter.serviceName == NULL ? "*" : opts->filter.serviceName;
filter = celix_serviceRegistry_createFilterFor(ctx->framework->registry, opts->filter.serviceName, opts->filter.versionRange, opts->filter.filter);
if(filter == NULL) {
celix_framework_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, __FUNCTION__, __BASE_FILE__, __LINE__,
filter = celix_serviceRegistry_createFilterFor(
ctx->framework->registry, opts->filter.serviceName, opts->filter.versionRange, opts->filter.filter);
if (filter == NULL) {
celix_framework_log(ctx->framework->logger,
CELIX_LOG_LEVEL_ERROR,
__FUNCTION__,
__BASE_FILE__,
__LINE__,
"Error cannot create filter.");
return NULL;
}
tracker = calloc(1, sizeof(*tracker));
if(tracker == NULL) {
if (tracker == NULL) {
free(filter);
celix_framework_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, __FUNCTION__, __BASE_FILE__, __LINE__,
celix_framework_log(ctx->framework->logger,
CELIX_LOG_LEVEL_ERROR,
__FUNCTION__,
__BASE_FILE__,
__LINE__,
"No memory for tracker.");
return NULL;
return NULL;
}

tracker->context = ctx;
tracker->serviceName = celix_utils_strdup(serviceName);
tracker->filter = filter;
tracker->state = CELIX_SERVICE_TRACKER_CLOSED;

//setting callbacks
// setting callbacks
tracker->callbackHandle = opts->callbackHandle;
tracker->set = opts->set;
tracker->add = opts->add;
Expand All @@ -684,8 +691,18 @@ celix_service_tracker_t* celix_serviceTracker_createWithOptions(
tracker->currentHighestServiceId = -1;

tracker->listener.handle = tracker;
tracker->listener.serviceChanged = (void *) serviceTracker_serviceChanged;
tracker->listener.serviceChanged = (void*)serviceTracker_serviceChanged;
return tracker;
}

celix_service_tracker_t* celix_serviceTracker_createWithOptions(
bundle_context_t *ctx,
const celix_service_tracking_options_t *opts
) {
celix_service_tracker_t *tracker = celix_serviceTracker_createClosedWithOptions(ctx, opts);
if(tracker == NULL) {
return NULL;
}
serviceTracker_open(tracker);
return tracker;
}
Expand Down
Loading