From 7428adaf0da600e9b80fc3857d24483656cb4f45 Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Mon, 15 May 2023 18:46:44 +0200 Subject: [PATCH] BUG/MINOR: hlua: SET_SAFE_LJMP misuse in hlua_event_runner() When hlua_event_runner() pauses the subscription (ie: if the consumer can't keep up the pace), hlua_traceback() is used to get the current lua trace (running context) to provide some info to the user. However, as hlua_traceback() may raise an error (__LJMP) is set, it is used within a SET_SAFE_LJMP() / RESET_SAFE_LJMP() combination to ensure lua errors are properly handled and don't result in unexpected behavior. But the current usage of SET_SAFE_LJMP() within the function is wrong since hlua_traceback() will run a second time (unprotected) if the first (protected) attempt fails. This is undefined behavior and could even lead to crashes. Hopefully it is very hard to trigger this code path, thus we can consider this as a minor bug. Also using this as an opportunity to enhance the message report to make it more meaningful to the user. This should fix GH #2159. It is a 2.8 specific bug, no backport needed unless c84899c636 ("MEDIUM: hlua/event_hdl: initial support for event handlers") gets backported. --- src/hlua.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/hlua.c b/src/hlua.c index 98ee44170..4ad23f2e2 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -9329,7 +9329,7 @@ static struct task *hlua_event_runner(struct task *task, void *context, unsigned const char *error = NULL; if (!hlua_sub->paused && event_hdl_async_equeue_size(&hlua_sub->equeue) > 100) { - const char *trace; + const char *trace = NULL; /* We reached the limit of pending events in the queue: we should * warn the user, and temporarily pause the subscription to give a chance @@ -9345,14 +9345,19 @@ static struct task *hlua_event_runner(struct task *task, void *context, unsigned event_hdl_pause(hlua_sub->sub); hlua_sub->paused = 1; - /* The following Lua calls can fail. */ - if (!SET_SAFE_LJMP(hlua_sub->hlua)) - trace = NULL; - trace = hlua_traceback(hlua_sub->hlua->T, ", "); - /* At this point the execution is safe. */ - RESET_SAFE_LJMP(hlua_sub->hlua); - - ha_warning("Lua event_hdl: pausing the subscription because the function fails to keep up the pace (%u unconsumed events): %s\n", event_hdl_async_equeue_size(&hlua_sub->equeue), ((trace) ? trace : "")); + if (SET_SAFE_LJMP(hlua_sub->hlua)) { + /* The following Lua call may fail. */ + trace = hlua_traceback(hlua_sub->hlua->T, ", "); + /* At this point the execution is safe. */ + RESET_SAFE_LJMP(hlua_sub->hlua); + } else { + /* Lua error was raised while fetching lua trace from current ctx */ + SEND_ERR(NULL, "Lua event_hdl: unexpected error (memory failure?).\n"); + } + ha_warning("Lua event_hdl: pausing the subscription because the handler fails " + "to keep up the pace (%u unconsumed events) from %s.\n", + event_hdl_async_equeue_size(&hlua_sub->equeue), + (trace) ? trace : "[unknown]"); } if (HLUA_IS_RUNNING(hlua_sub->hlua)) {