BUG/MEDIUM: hlua: improper lock usage with SET_SAFE_LJMP()
When we want to perform some unsafe lua stack manipulations from an
unprotected lua environment, we use SET_SAFE_LJMP() RESET_SAFE_LJMP()
combination to lock lua stack and catch potential lua exceptions that
may occur between the two.
Hence, we regularly find this pattern (duplicated over and over):
|if (!SET_SAFE_LJMP(hlua)) {
| const char *error;
|
| if (lua_type(hlua->T, -1) == LUA_TSTRING)
| error = hlua_tostring_safe(hlua->T, -1);
| else
| error = "critical error";
| SEND_ERR(NULL, "*: %s.\n", error);
|}
This is wrong because when SET_SAFE_LJMP() returns false (meaning that an
exception was caught), then the lua lock was released already, thus the
caller is not expected to perform lua stack manipulations (because the
main lua stack may be shared between multiple threads). In the pattern
above we only want to retrieve the lua exception message which may be
found at the top of the stack, to do so we now explicitly take the lua
lock before accessing the lua stack. Note that hlua_lock() doesn't catch
lua exceptions so only safe lua functions are expected to be used there
(lua functions that may NOT raise exceptions).
It should be backported to every stable versions.
[ada: some ctx adj will be required for older versions as event_hdl
doesn't exist prior to 2.8 and filters were implemented in 2.5, thus
some chunks won't apply, but other fixes should stay relevant]
(cherry picked from commit 19b016f9f8b921b40f5ab0496254f7cbb9103488)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 63cfa3ea8be50146e407c098d451eeee14d67abe)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/hlua.c b/src/hlua.c
index 4cfb768..5818e51 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -9624,11 +9624,13 @@
/* The following Lua calls can fail. */
if (!SET_SAFE_LJMP(hlua_sub->hlua)) {
+ hlua_lock(hlua_sub->hlua);
if (lua_type(hlua_sub->hlua->T, -1) == LUA_TSTRING)
error = hlua_tostring_safe(hlua_sub->hlua->T, -1);
else
error = "critical error";
ha_alert("Lua event_hdl: %s.\n", error);
+ hlua_unlock(hlua_sub->hlua);
goto skip_event;
}
@@ -9919,11 +9921,13 @@
/* The following Lua calls can fail. */
if (!SET_SAFE_LJMP(stream->hlua)) {
+ hlua_lock(stream->hlua);
if (lua_type(stream->hlua->T, -1) == LUA_TSTRING)
error = hlua_tostring_safe(stream->hlua->T, -1);
else
error = "critical error";
SEND_ERR(stream->be, "Lua converter '%s': %s.\n", fcn->name, error);
+ hlua_unlock(stream->hlua);
return 0;
}
@@ -10044,11 +10048,13 @@
/* The following Lua calls can fail. */
if (!SET_SAFE_LJMP(stream->hlua)) {
+ hlua_lock(stream->hlua);
if (lua_type(stream->hlua->T, -1) == LUA_TSTRING)
error = hlua_tostring_safe(stream->hlua->T, -1);
else
error = "critical error";
SEND_ERR(smp->px, "Lua sample-fetch '%s': %s.\n", fcn->name, error);
+ hlua_unlock(stream->hlua);
return 0;
}
@@ -10374,12 +10380,14 @@
/* The following Lua calls can fail. */
if (!SET_SAFE_LJMP(s->hlua)) {
+ hlua_lock(s->hlua);
if (lua_type(s->hlua->T, -1) == LUA_TSTRING)
error = hlua_tostring_safe(s->hlua->T, -1);
else
error = "critical error";
SEND_ERR(px, "Lua function '%s': %s.\n",
rule->arg.hlua_rule->fcn->name, error);
+ hlua_unlock(s->hlua);
goto end;
}
@@ -10560,12 +10568,14 @@
/* The following Lua calls can fail. */
if (!SET_SAFE_LJMP(hlua)) {
+ hlua_lock(hlua);
if (lua_type(hlua->T, -1) == LUA_TSTRING)
error = hlua_tostring_safe(hlua->T, -1);
else
error = "critical error";
SEND_ERR(strm->be, "Lua applet tcp '%s': %s.\n",
ctx->rule->arg.hlua_rule->fcn->name, error);
+ hlua_unlock(hlua);
return -1;
}
@@ -10751,12 +10761,14 @@
/* The following Lua calls can fail. */
if (!SET_SAFE_LJMP(hlua)) {
+ hlua_lock(hlua);
if (lua_type(hlua->T, -1) == LUA_TSTRING)
error = hlua_tostring_safe(hlua->T, -1);
else
error = "critical error";
SEND_ERR(strm->be, "Lua applet http '%s': %s.\n",
ctx->rule->arg.hlua_rule->fcn->name, error);
+ hlua_unlock(hlua);
return -1;
}
@@ -11386,11 +11398,13 @@
/* The following Lua calls can fail. */
if (!SET_SAFE_LJMP(hlua)) {
+ hlua_lock(hlua);
if (lua_type(hlua->T, -1) == LUA_TSTRING)
error = hlua_tostring_safe(hlua->T, -1);
else
error = "critical error";
SEND_ERR(NULL, "Lua cli '%s': %s.\n", fcn->name, error);
+ hlua_unlock(hlua);
goto error;
}
@@ -11843,11 +11857,13 @@
if (!SET_SAFE_LJMP(s->hlua)) {
const char *error;
+ hlua_lock(s->hlua);
if (lua_type(s->hlua->T, -1) == LUA_TSTRING)
error = hlua_tostring_safe(s->hlua->T, -1);
else
error = "critical error";
SEND_ERR(s->be, "Lua filter '%s': %s.\n", conf->reg->name, error);
+ hlua_unlock(s->hlua);
ret = 0;
goto end;
}
@@ -11979,11 +11995,13 @@
if (!SET_SAFE_LJMP(flt_hlua)) {
const char *error;
+ hlua_lock(flt_hlua);
if (lua_type(flt_hlua->T, -1) == LUA_TSTRING)
error = hlua_tostring_safe(flt_hlua->T, -1);
else
error = "critical error";
SEND_ERR(s->be, "Lua filter '%s': %s.\n", conf->reg->name, error);
+ hlua_unlock(flt_hlua);
goto end;
}