Add sanity check in unlang_module_yield

Why?  Because if the yield is done _AFTER_ a function has been pushed onto the stack, we get undefined reuslts

diff --git a/src/lib/unlang/module.c b/src/lib/unlang/module.c
index 7586403a31adbc5e8bfe1eac16cf5d7d9ba24b5c..f6d84031d0def0d92abb7d01b8a039063bf9fa52 100644 (file)
--- a/src/lib/unlang/module.c
+++ b/src/lib/unlang/module.c
@@ -429,6 +429,7 @@ unlang_action_t     unlang_module_yield_to_retry(request_t *request, module_method_t
  * @param[in] rctx             to pass to the callbacks.
  * @return
  *     - UNLANG_ACTION_YIELD.
+ *     - UNLANG_ACTION_FAIL if this is not a module frame.
  */
 unlang_action_t unlang_module_yield(request_t *request,
                                    module_method_t resume, unlang_module_signal_t signal, fr_signal_t sigmask, void *rctx)
@@ -439,6 +440,11 @@ unlang_action_t unlang_module_yield(request_t *request,
 
        REQUEST_VERIFY(request);        /* Check the yielded request is sane */
 
+       if (frame->instruction->type != UNLANG_TYPE_MODULE) {
+               fr_assert_msg(0, "unlang_module_yield called on a non-module frame");
+               return UNLANG_ACTION_FAIL;
+       }
+
        state->rctx = rctx;
        state->resume = resume;
 

diff --git a/src/modules/rlm_ldap/rlm_ldap.c b/src/modules/rlm_ldap/rlm_ldap.c
index 0dcebf5be651ed8bd051905015a6853350fb8239..aad39441d9cd9bbc5d78783fd081769c96c1209b 100644 (file)
--- a/src/modules/rlm_ldap/rlm_ldap.c
+++ b/src/modules/rlm_ldap/rlm_ldap.c
@@ -1606,7 +1606,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authenticate(unlang_result_t *p_resu
 }
 
 #define REPEAT_MOD_AUTHORIZE_RESUME \
-       if (unlang_module_set_resume(request, mod_authorize_resume) < 0) do { \
+       if (unlang_module_yield(request, mod_authorize_resume, NULL, 0, autz_ctx) == UNLANG_ACTION_FAIL) do { \
                p_result->rcode = RLM_MODULE_FAIL; \
                goto finish; \
        } while (0)

diff --git a/src/modules/rlm_mschap/rlm_mschap.c b/src/modules/rlm_mschap/rlm_mschap.c
index 519e6b9562e9485f9986c373aff2bba5e2b16917..ac6ae5af39783e4aa4ab2bb60f9bdb77e913f5e5 100644 (file)
--- a/src/modules/rlm_mschap/rlm_mschap.c
+++ b/src/modules/rlm_mschap/rlm_mschap.c
@@ -2305,7 +2305,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authenticate(unlang_result_t *p_resu
 #ifdef WITH_TLS
                        if (mschap_new_pass_decrypt(request, auth_ctx) < 0) RETURN_UNLANG_FAIL;
 
-                       if (unlang_module_yield(request, mod_authenticate_resume, NULL, 0, auth_ctx) != UNLANG_ACTION_YIELD) {
+                       if (unlang_module_yield(request, mod_authenticate_resume, NULL, 0, auth_ctx) == UNLANG_ACTION_FAIL) {
                                RETURN_UNLANG_FAIL;
                        }
 
@@ -2327,7 +2327,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authenticate(unlang_result_t *p_resu
                        /*
                         *      Run the resumption function where we're done with:
                         */
-                       if (unlang_module_yield(request, mod_authenticate_resume, NULL, 0, auth_ctx) != UNLANG_ACTION_YIELD) {
+                       if (unlang_module_yield(request, mod_authenticate_resume, NULL, 0, auth_ctx) == UNLANG_ACTION_FAIL) {
                                RETURN_UNLANG_FAIL;
                        };
 

diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c
index aa9ed594cfa5dab46c5f969b14545e9c008148ad..72f3847b7473bf58a0bf87cc42257face9088c6f 100644 (file)
--- a/src/modules/rlm_sql/rlm_sql.c
+++ b/src/modules/rlm_sql/rlm_sql.c
@@ -1292,7 +1292,7 @@ static unlang_action_t CC_HINT(nonnull)  mod_autz_group_resume(unlang_result_t *
 
        switch(autz_ctx->status) {
        case SQL_AUTZ_GROUP_MEMB:
-               if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, mctx->rctx) != UNLANG_ACTION_YIELD) RETURN_UNLANG_FAIL;
+               if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, mctx->rctx) == UNLANG_ACTION_FAIL) RETURN_UNLANG_FAIL;
                MEM(autz_ctx->group_ctx = talloc(autz_ctx, sql_group_ctx_t));
                *autz_ctx->group_ctx = (sql_group_ctx_t) {
                        .inst = inst,
@@ -1341,7 +1341,7 @@ static unlang_action_t CC_HINT(nonnull)  mod_autz_group_resume(unlang_result_t *
                }
 
                if (call_env->group_check_query) {
-                       if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, mctx->rctx) != UNLANG_ACTION_YIELD) RETURN_UNLANG_FAIL;
+                       if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, mctx->rctx) == UNLANG_ACTION_FAIL) RETURN_UNLANG_FAIL;
                        if (unlang_tmpl_push(autz_ctx, &autz_ctx->query, request,
                                             call_env->group_check_query, NULL) < 0) RETURN_UNLANG_FAIL;
                        return UNLANG_ACTION_PUSHED_CHILD;
@@ -1361,7 +1361,7 @@ static unlang_action_t CC_HINT(nonnull)  mod_autz_group_resume(unlang_result_t *
                        .query = query,
                };
 
-               if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, mctx->rctx) != UNLANG_ACTION_YIELD) RETURN_UNLANG_FAIL;
+               if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, mctx->rctx) == UNLANG_ACTION_FAIL) RETURN_UNLANG_FAIL;
                if (sql_get_map_list(request, map_ctx, autz_ctx->trunk) == UNLANG_ACTION_PUSHED_CHILD) {
                        autz_ctx->status = autz_ctx->status & SQL_AUTZ_STAGE_GROUP ? SQL_AUTZ_GROUP_CHECK_RESUME : SQL_AUTZ_PROFILE_CHECK_RESUME;
                        return UNLANG_ACTION_PUSHED_CHILD;
@@ -1395,7 +1395,7 @@ static unlang_action_t CC_HINT(nonnull)  mod_autz_group_resume(unlang_result_t *
 
                if (call_env->group_reply_query) {
                group_reply_push:
-                       if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, mctx->rctx) != UNLANG_ACTION_YIELD) RETURN_UNLANG_FAIL;
+                       if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, mctx->rctx) == UNLANG_ACTION_FAIL) RETURN_UNLANG_FAIL;
                        if (unlang_tmpl_push(autz_ctx, &autz_ctx->query, request,
                                             call_env->group_reply_query, NULL) < 0) RETURN_UNLANG_FAIL;
                        autz_ctx->status = autz_ctx->status & SQL_AUTZ_STAGE_GROUP ? SQL_AUTZ_GROUP_REPLY : SQL_AUTZ_PROFILE_REPLY;
@@ -1417,7 +1417,7 @@ static unlang_action_t CC_HINT(nonnull)  mod_autz_group_resume(unlang_result_t *
                        .expand_rhs = true,
                };
 
-               if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, mctx->rctx) != UNLANG_ACTION_YIELD) RETURN_UNLANG_FAIL;
+               if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, mctx->rctx) == UNLANG_ACTION_FAIL) RETURN_UNLANG_FAIL;
                if (sql_get_map_list(request, map_ctx, autz_ctx->trunk) == UNLANG_ACTION_PUSHED_CHILD) {
                        autz_ctx->status = autz_ctx->status & SQL_AUTZ_STAGE_GROUP ? SQL_AUTZ_GROUP_REPLY_RESUME : SQL_AUTZ_PROFILE_REPLY_RESUME;
                        return UNLANG_ACTION_PUSHED_CHILD;
@@ -1531,7 +1531,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize_resume(unlang_result_t *p_
                        .query = query,
                };
 
-               if (unlang_module_yield(request, mod_authorize_resume, NULL, 0, autz_ctx) != UNLANG_ACTION_YIELD) RETURN_UNLANG_FAIL;
+               if (unlang_module_yield(request, mod_authorize_resume, NULL, 0, autz_ctx) == UNLANG_ACTION_FAIL) RETURN_UNLANG_FAIL;
                if (sql_get_map_list(request, map_ctx, autz_ctx->trunk) == UNLANG_ACTION_PUSHED_CHILD){
                        autz_ctx->status = SQL_AUTZ_CHECK_RESUME;
                        return UNLANG_ACTION_PUSHED_CHILD;
@@ -1558,7 +1558,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize_resume(unlang_result_t *p_
 
                if (!call_env->reply_query) goto skip_reply;
 
-               if (unlang_module_yield(request, mod_authorize_resume, NULL, 0, autz_ctx) != UNLANG_ACTION_YIELD) RETURN_UNLANG_FAIL;
+               if (unlang_module_yield(request, mod_authorize_resume, NULL, 0, autz_ctx) == UNLANG_ACTION_FAIL) RETURN_UNLANG_FAIL;
                if (unlang_tmpl_push(autz_ctx, &autz_ctx->query, request, call_env->reply_query, NULL) < 0) RETURN_UNLANG_FAIL;
                autz_ctx->status = SQL_AUTZ_REPLY;
                return UNLANG_ACTION_PUSHED_CHILD;
@@ -1573,7 +1573,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize_resume(unlang_result_t *p_
                        .expand_rhs = true,
                };
 
-               if (unlang_module_yield(request, mod_authorize_resume, NULL, 0, autz_ctx) != UNLANG_ACTION_YIELD) RETURN_UNLANG_FAIL;
+               if (unlang_module_yield(request, mod_authorize_resume, NULL, 0, autz_ctx) == UNLANG_ACTION_FAIL) RETURN_UNLANG_FAIL;
                if (sql_get_map_list(request, map_ctx, autz_ctx->trunk) == UNLANG_ACTION_PUSHED_CHILD){
                        autz_ctx->status = SQL_AUTZ_REPLY_RESUME;
                        return UNLANG_ACTION_PUSHED_CHILD;
@@ -1620,7 +1620,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize_resume(unlang_result_t *p_
                                break;
                        }
 
-                       if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, autz_ctx) != UNLANG_ACTION_YIELD) RETURN_UNLANG_FAIL;
+                       if (unlang_module_yield(request, mod_autz_group_resume, NULL, 0, autz_ctx) == UNLANG_ACTION_FAIL) RETURN_UNLANG_FAIL;
                        if (unlang_tmpl_push(autz_ctx, &autz_ctx->query, request,
                                             call_env->membership_query, NULL) < 0) RETURN_UNLANG_FAIL;
                        autz_ctx->status = SQL_AUTZ_GROUP_MEMB;
@@ -1692,7 +1692,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize(unlang_result_t *p_result,
 
        if (unlang_module_yield(request,
                                (call_env->check_query || call_env->reply_query) ? mod_authorize_resume : mod_autz_group_resume,
-                               NULL, 0, autz_ctx) != UNLANG_ACTION_YIELD) {
+                               NULL, 0, autz_ctx) == UNLANG_ACTION_FAIL) {
        error:
                talloc_free(autz_ctx);
                RETURN_UNLANG_FAIL;
@@ -1814,7 +1814,7 @@ next:
        talloc_free(query_ctx);
        redundant_ctx->query_no++;
        if (redundant_ctx->query_no >= talloc_array_length(call_env->query)) RETURN_UNLANG_NOOP;
-       if (unlang_module_yield(request, mod_sql_redundant_resume, NULL, 0, redundant_ctx) < 0) RETURN_UNLANG_FAIL;
+       if (unlang_module_yield(request, mod_sql_redundant_resume, NULL, 0, redundant_ctx) == UNLANG_ACTION_FAIL) RETURN_UNLANG_FAIL;
        if (unlang_tmpl_push(redundant_ctx, &redundant_ctx->query, request, call_env->query[redundant_ctx->query_no], NULL) < 0) RETURN_UNLANG_FAIL;
 
        RDEBUG2("Trying next query...");

diff --git a/src/modules/rlm_sqlcounter/rlm_sqlcounter.c b/src/modules/rlm_sqlcounter/rlm_sqlcounter.c
index 5a3c481cf94cc1ce50c8799fde370fa430863428..42a588640a97eda44e6322a6c5fec42d3b4aed5a 100644 (file)
--- a/src/modules/rlm_sqlcounter/rlm_sqlcounter.c
+++ b/src/modules/rlm_sqlcounter/rlm_sqlcounter.c
@@ -430,7 +430,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize(unlang_result_t *p_result,
                .limit = limit
        };
 
-       if (unlang_module_yield(request, mod_authorize_resume, NULL, 0, rctx) != UNLANG_ACTION_YIELD) {
+       if (unlang_module_yield(request, mod_authorize_resume, NULL, 0, rctx) == UNLANG_ACTION_FAIL) {
        error:
                talloc_free(rctx);
                RETURN_UNLANG_FAIL;

diff --git a/src/modules/rlm_sqlippool/rlm_sqlippool.c b/src/modules/rlm_sqlippool/rlm_sqlippool.c
index e87e6f555f04979e758b8a34a5ec8997482c0fc1..93a821f3a226ef464f0e4d3257e1a07d678d7cf7 100644 (file)
--- a/src/modules/rlm_sqlippool/rlm_sqlippool.c
+++ b/src/modules/rlm_sqlippool/rlm_sqlippool.c
@@ -225,7 +225,7 @@ static int sqlippool_alloc_ctx_free(ippool_alloc_ctx_t *to_free)
        return 0;
 }
 
-#define REPEAT_MOD_ALLOC_RESUME if (unlang_module_yield(request, mod_alloc_resume, NULL, 0, mctx->rctx) < 0) RETURN_UNLANG_FAIL
+#define REPEAT_MOD_ALLOC_RESUME if (unlang_module_yield(request, mod_alloc_resume, NULL, 0, mctx->rctx) == UNLANG_ACTION_FAIL) RETURN_UNLANG_FAIL
 #define SUBMIT_QUERY(_query_str, _new_status, _type, _function) do { \
        alloc_ctx->status = _new_status; \
        REPEAT_MOD_ALLOC_RESUME; \