Hello everyone,
While working on a ulogd2 deployment, i built a stack that consists in:
- A "home-made" INPUT plugin that launchs a pcap capture upon
start() and passes a fd to ulogd2.
- Some regular ulogd2 filters
- JSON output plugin that writes to a Unix socket.
Everything works fine, except if unix socket is not available.
In that case, when the stack is started at launch, the JSON output
start() function returns -1 and ulogd.c consider that it fails
starting the stack and then destroy it.
Unfortunately, ulogd.c only free() the stack context without calling
stop() functions on plugins that were correctly start()-ed.
In my case, it results in my pcap input triggering the stack even if
it was destroyed.
This ends with a segmentation fault for the ulogd process.
I think we should consider calling stop() functions before destroying the stack.
A patch implementing this is attached (for version 2.0.7, but this
part of the code didn't changed on latest).
Best regards,
--
Gerald Colangelo
--- ulogd2-2.0.7/src/ulogd.c 2018-04-27 01:10:42.316872034 +0200
+++ ulogd2-2.0.7-patched/src/ulogd.c 2023-12-14 18:04:06.994661722 +0100
@@ -931,7 +931,7 @@
static int create_stack_start_instances(struct ulogd_pluginstance_stack *stack)
{
int ret;
- struct ulogd_pluginstance *pi;
+ struct ulogd_pluginstance *pi, *stop;
/* start from input to output plugin */
llist_for_each_entry(pi, &stack->list, list) {
@@ -945,11 +945,27 @@
ulogd_log(ULOGD_ERROR,
"error starting `%s'\n",
pi->id);
- return ret;
+ goto cleanup_fail;
}
}
}
return 0;
+cleanup_fail:
+ stop = pi;
+ llist_for_each_entry(pi, &stack->list, list) {
+ if (pi == stop)
+ /* the one that failed, stops the cleanup here */
+ break;
+ if (!pi->plugin->stop)
+ continue;
+ ret = pi->plugin->stop(pi);
+ if (ret < 0) {
+ ulogd_log(ULOGD_ERROR,
+ "error stopping `%s'\n",
+ pi->id);
+ }
+ }
+ return -1;
}
/* create a new stack of plugins */
