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]

diff --git a/src/hlua.c b/src/hlua.c
index 2339711a38af2f7b372f1107a458f247feb85cea..216d73a39cad02b594880ab6500585d57257d73f 100644 (file)
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -9649,11 +9649,13 @@ static struct task *hlua_event_runner(struct task *task, void *context, unsigned
 
                /* 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;
                }
 
@@ -9944,11 +9946,13 @@ static int hlua_sample_conv_wrapper(const struct arg *arg_p, struct sample *smp,
 
                /* 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;
                }
 
@@ -10069,11 +10073,13 @@ static int hlua_sample_fetch_wrapper(const struct arg *arg_p, struct sample *smp
 
                /* 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;
                }
 
@@ -10399,12 +10405,14 @@ static enum act_return hlua_action(struct act_rule *rule, struct proxy *px,
 
                /* 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;
                }
 
@@ -10585,12 +10593,14 @@ static int hlua_applet_tcp_init(struct appctx *ctx)
 
        /* 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;
        }
 
@@ -10776,12 +10786,14 @@ static int hlua_applet_http_init(struct appctx *ctx)
 
        /* 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;
        }
 
@@ -11416,11 +11428,13 @@ static int hlua_cli_parse_fct(char **args, char *payload, struct appctx *appctx,
 
        /* 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;
        }
 
@@ -11873,11 +11887,13 @@ static int hlua_filter_new(struct stream *s, struct filter *filter)
                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;
                }
@@ -12009,11 +12025,13 @@ static int hlua_filter_callback(struct stream *s, struct filter *filter, const c
                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;
                }