Tim,
I've just added the following patch after finding another affected place
in smp_fetch_sc_trackers(). Fortunately this one couldn't happen, but the
checks on the code path definitely are wrong for the same reaons as in the
two other ones. Thus I prefer to make stktable_release() safe against a
NULL argument.
William, I marked it as a possible candidate for backporting to 1.8. I
think it doesn't hurt to make the code safer after these last two bugs.
Cheers,
Willy
----
>From 43e903553edc94bb9b33e965f37d8d218f7d1482 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Wed, 27 Jun 2018 06:25:57 +0200
Subject: MINOR: stick-tables: make stktable_release() do nothing on NULL
stktable_release() has been involved in two recent crashes by being
used without enough care. Just like any free() function this one is
often called on an exit path with a possibly unsafe argument. Given
that there is another case (smp_fetch_sc_trackers()) which theorically
could call it with an unchecked NULL, though it cannot happen since
the function doesn't support being called with src_* hence cannot make
use of tmpstkctr, let's rather move the check into the function itself
to make it safer for the long term.
This patch could be backported to 1.8 as a strengthening measure.
---
src/stick_table.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/stick_table.c b/src/stick_table.c
index 8e16830..3c4e783 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -412,9 +412,11 @@ void stktable_touch_local(struct stktable *t, struct
stksess *ts, int decrefcnt)
ts->ref_cnt--;
HA_SPIN_UNLOCK(STK_TABLE_LOCK, &t->lock);
}
-/* Just decrease the ref_cnt of the current session */
-void stktable_release(struct stktable *t, struct stksess *ts)
+/* Just decrease the ref_cnt of the current session. Does nothing if <ts> is
NULL */
+static void stktable_release(struct stktable *t, struct stksess *ts)
{
+ if (!ts)
+ return;
HA_SPIN_LOCK(STK_TABLE_LOCK, &t->lock);
ts->ref_cnt--;
HA_SPIN_UNLOCK(STK_TABLE_LOCK, &t->lock);
@@ -875,8 +877,7 @@ static int sample_conv_in_table(const struct arg *arg_p,
struct sample *smp, voi
smp->data.type = SMP_T_BOOL;
smp->data.u.sint = !!ts;
smp->flags = SMP_F_VOL_TEST;
- if (ts)
- stktable_release(t, ts);
+ stktable_release(t, ts);
return 1;
}
@@ -2014,7 +2015,7 @@ smp_fetch_sc_tracked(const struct arg *args, struct
sample *smp, const char *kw,
smp->data.u.sint = !!stkctr;
/* release the ref count */
- if ((stkctr == &tmpstkctr) && stkctr_entry(stkctr))
+ if ((stkctr == &tmpstkctr))
stktable_release(stkctr->table, stkctr_entry(stkctr));
return 1;
--
1.7.12.1
