BUG/MEDIUM: stick-tables: Don't let table_process_entry() handle refcnt
authorOlivier Houchard <ohouchard@haproxy.com>
Thu, 11 Sep 2025 16:22:34 +0000 (18:22 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Wed, 1 Oct 2025 14:48:35 +0000 (16:48 +0200)
Instead of having table_process_entry() decrement the session's ref
counter, do it outside, from the caller. Some were missed, such as when
an action was invalid, which would lead to the ref counter not being
decremented, and the session not being destroyable.
It makes more sense to do that from the caller, who just obtained the
ref counter, anyway.
This should be backporter up to 2.8.

(cherry picked from commit 71199e394c5e63fda7070cd7ae5b4b691411259d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 1b2a06b994c0d537ca2da0af101c2b84735d67f9)
[cf: ctx adjt]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit e9c299c31d67bebe68c22503a98b51da485cdca7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

src/stick_table.c

index 81b71f4..2fc3c56 100644 (file)
@@ -5369,10 +5369,11 @@ struct show_table_ctx {
 /* Processes a single table entry <ts>.
  * returns 0 if it wants to be called again, 1 if has ended processing.
  */
-static int table_process_entry(struct appctx *appctx, struct stksess *ts, char **args)
+static int table_process_entry(struct appctx *appctx, struct stksess **tsptr, char **args)
 {
        struct show_table_ctx *ctx = appctx->svcctx;
        struct stktable *t = ctx->target;
+       struct stksess *ts = *tsptr;
        long long value;
        int data_type;
        int cur_arg;
@@ -5409,20 +5410,22 @@ static int table_process_entry(struct appctx *appctx, struct stksess *ts, char *
        case STK_CLI_ACT_SHOW:
                chunk_reset(&trash);
                if (!table_dump_head_to_buffer(&trash, appctx, t, t)) {
-                       stktable_release(t, ts);
                        return 0;
                }
                HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
                if (!table_dump_entry_to_buffer(&trash, appctx, t, ts)) {
                        HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
-                       stktable_release(t, ts);
                        return 0;
                }
                HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
-               stktable_release(t, ts);
                break;
 
        case STK_CLI_ACT_CLR:
+               /*
+                * Now matter if we managed to kill the stksess or not,
+                * the ref_cnt will be decremented, so let the caller know.
+                */
+               *tsptr = NULL;
                if (!stksess_kill(t, ts, 1)) {
                        /* don't delete an entry which is currently referenced */
                        return cli_err(appctx, "Entry currently in use, cannot remove\n");
@@ -5435,7 +5438,7 @@ static int table_process_entry(struct appctx *appctx, struct stksess *ts, char *
                        if (strncmp(args[cur_arg], "data.", 5) != 0) {
                                cli_err(appctx, "\"data.<type>\" followed by a value expected\n");
                                HA_RWLOCK_WRUNLOCK(STK_SESS_LOCK, &ts->lock);
-                               stktable_touch_local(t, ts, 1);
+                               stktable_touch_local(t, ts, 0);
                                return 1;
                        }
 
@@ -5443,21 +5446,21 @@ static int table_process_entry(struct appctx *appctx, struct stksess *ts, char *
                        if (data_type < 0) {
                                cli_err(appctx, "Unknown data type\n");
                                HA_RWLOCK_WRUNLOCK(STK_SESS_LOCK, &ts->lock);
-                               stktable_touch_local(t, ts, 1);
+                               stktable_touch_local(t, ts, 0);
                                return 1;
                        }
 
                        if (!t->data_ofs[data_type]) {
                                cli_err(appctx, "Data type not stored in this table\n");
                                HA_RWLOCK_WRUNLOCK(STK_SESS_LOCK, &ts->lock);
-                               stktable_touch_local(t, ts, 1);
+                               stktable_touch_local(t, ts, 0);
                                return 1;
                        }
 
                        if (!*args[cur_arg+1] || strl2llrc(args[cur_arg+1], strlen(args[cur_arg+1]), &value) != 0) {
                                cli_err(appctx, "Require a valid integer value to store\n");
                                HA_RWLOCK_WRUNLOCK(STK_SESS_LOCK, &ts->lock);
-                               stktable_touch_local(t, ts, 1);
+                               stktable_touch_local(t, ts, 0);
                                return 1;
                        }
 
@@ -5491,7 +5494,7 @@ static int table_process_entry(struct appctx *appctx, struct stksess *ts, char *
                        }
                }
                HA_RWLOCK_WRUNLOCK(STK_SESS_LOCK, &ts->lock);
-               stktable_touch_local(t, ts, 1);
+               stktable_touch_local(t, ts, 0);
                break;
 
        default:
@@ -5510,6 +5513,7 @@ static int table_process_entry_per_key(struct appctx *appctx, char **args)
        struct stktable *t = ctx->target;
        struct stksess *ts;
        struct sample key;
+       int ret;
 
        if (!*args[4])
                return cli_err(appctx, "Key value expected\n");
@@ -5547,7 +5551,10 @@ static int table_process_entry_per_key(struct appctx *appctx, char **args)
        } else
                ts = stktable_lookup_key(t, &static_table_key);
 
-       return table_process_entry(appctx, ts, args);
+       ret = table_process_entry(appctx, &ts, args);
+       if (ts)
+               stktable_release(t, ts);
+       return ret;
 }
 
 /* Processes a single table entry matching a specific ptr passed in argument.
@@ -5560,6 +5567,7 @@ static int table_process_entry_per_ptr(struct appctx *appctx, char **args)
        ulong ptr;
        char *error;
        struct stksess *ts;
+       int ret;
 
        if (!*args[4] || args[4][0] != '0' || args[4][1] != 'x')
                return cli_err(appctx, "Pointer expected (0xffff notation)\n");
@@ -5573,7 +5581,10 @@ static int table_process_entry_per_ptr(struct appctx *appctx, char **args)
        if (!ts)
                return cli_err(appctx, "No entry can be found matching ptr.\n");
 
-       return table_process_entry(appctx, ts, args);
+       ret = table_process_entry(appctx, &ts, args);
+       if (ts)
+               stktable_release(t, ts);
+       return ret;
 }
 
 /* Prepares the appctx fields with the data-based filters from the command line.