RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2025-02-23 06:06:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

BUG/MEDIUM: cli: make "show sess" really thread-safe

Browse Source 
This one used to rely on a few spin locks around lists manipulations
only but 1) there were still a few races (e.g. when aborting, or
between STAT_ST_INIT and STAT_ST_LIST), and 2) after last commit
which dumps htx info it became obvious that dereferencing the buffer
contents is not safe at all.

This patch uses the thread isolation from the rendez-vous point
instead, to guarantee that nothing moves during the dump. It may
make the dump a bit slower but it will be 100% safe.

This fix must be backported to 1.9, and possibly to 1.8 which likely
suffers from the short races above, eventhough they're extremely
hard to trigger.
This commit is contained in:
Willy Tarreau 2019-01-04 17:42:57 +01:00
parent f1b11e2d16
commit e6e52366c1
1 changed files with 27 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
src/stream.c
View File

@ -2812,13 +2812,9 @@ static int stats_dump_full_strm_to_buffer(struct stream_interface *si, struct st
if (appctx->ctx.sess.section > 0 && appctx->ctx.sess.uid != strm->uniq_id) {
/* stream changed, no need to go any further */
chunk_appendf(&trash, " *** session terminated while we were watching it ***\n");
if (ci_putchk(si_ic(si), &trash) == -1) {
si_rx_room_blk(si);
return 0;
}
appctx->ctx.sess.uid = 0;
appctx->ctx.sess.section = 0;
return 1;
if (ci_putchk(si_ic(si), &trash) == -1)
goto full;
goto done;
}
switch (appctx->ctx.sess.section) {
@ -3136,17 +3132,18 @@ static int stats_dump_full_strm_to_buffer(struct stream_interface *si, struct st
(unsigned long long)htx->extra);
}
if (ci_putchk(si_ic(si), &trash) == -1) {
si_rx_room_blk(si);
return 0;
}
if (ci_putchk(si_ic(si), &trash) == -1)
goto full;
/* use other states to dump the contents */
}
/* end of dump */
done:
appctx->ctx.sess.uid = 0;
appctx->ctx.sess.section = 0;
return 1;
full:
return 0;
}
@ -3169,14 +3166,16 @@ static int cli_parse_show_sess(char **args, char *payload, struct appctx *appctx
/* This function dumps all streams' states onto the stream interface's
* read buffer. It returns 0 if the output buffer is full and it needs
* to be called again, otherwise non-zero. It is designed to be called
* from stats_dump_sess_to_buffer() below.
* to be called again, otherwise non-zero. It proceeds in an isolated
* thread so there is no thread safety issue here.
*/
static int cli_io_handler_dump_sess(struct appctx *appctx)
{
struct stream_interface *si = appctx->owner;
struct connection *conn;
thread_isolate();
if (unlikely(si_ic(si)->flags & (CF_WRITE_ERROR|CF_SHUTW))) {
/* If we're forced to shut down, we might have to remove our
* reference to the last stream being dumped.
@ -3187,7 +3186,7 @@ static int cli_io_handler_dump_sess(struct appctx *appctx)
LIST_INIT(&appctx->ctx.sess.bref.users);
}
}
return 1;
goto done;
}
chunk_reset(&trash);
@ -3202,14 +3201,11 @@ static int cli_io_handler_dump_sess(struct appctx *appctx)
* pointer points back to the head of the streams list.
*/
LIST_INIT(&appctx->ctx.sess.bref.users);
HA_SPIN_LOCK(STRMS_LOCK, &streams_lock);
appctx->ctx.sess.bref.ref = streams.n;
HA_SPIN_UNLOCK(STRMS_LOCK, &streams_lock);
appctx->st2 = STAT_ST_LIST;
/* fall through */
case STAT_ST_LIST:
HA_SPIN_LOCK(STRMS_LOCK, &streams_lock);
/* first, let's detach the back-ref from a possible previous stream */
if (!LIST_ISEMPTY(&appctx->ctx.sess.bref.users)) {
LIST_DEL(&appctx->ctx.sess.bref.users);
@ -3229,10 +3225,8 @@ static int cli_io_handler_dump_sess(struct appctx *appctx)
LIST_ADDQ(&curr_strm->back_refs, &appctx->ctx.sess.bref.users);
/* call the proper dump() function and return if we're missing space */
if (!stats_dump_full_strm_to_buffer(si, curr_strm)) {
HA_SPIN_UNLOCK(STRMS_LOCK, &streams_lock);
return 0;
}
if (!stats_dump_full_strm_to_buffer(si, curr_strm))
goto full;
/* stream dump complete */
LIST_DEL(&appctx->ctx.sess.bref.users);
@ -3357,10 +3351,8 @@ static int cli_io_handler_dump_sess(struct appctx *appctx)
/* let's try again later from this stream. We add ourselves into
* this stream's users so that it can remove us upon termination.
*/
si_rx_room_blk(si);
LIST_ADDQ(&curr_strm->back_refs, &appctx->ctx.sess.bref.users);
HA_SPIN_UNLOCK(STRMS_LOCK, &streams_lock);
return 0;
goto full;
}
next_sess:
@ -3374,25 +3366,26 @@ static int cli_io_handler_dump_sess(struct appctx *appctx)
else
chunk_appendf(&trash, "Session not found.\n");
if (ci_putchk(si_ic(si), &trash) == -1) {
si_rx_room_blk(si);
HA_SPIN_UNLOCK(STRMS_LOCK, &streams_lock);
return 0;
}
if (ci_putchk(si_ic(si), &trash) == -1)
goto full;
appctx->ctx.sess.target = NULL;
appctx->ctx.sess.uid = 0;
HA_SPIN_UNLOCK(STRMS_LOCK, &streams_lock);
return 1;
goto done;
}
HA_SPIN_UNLOCK(STRMS_LOCK, &streams_lock);
/* fall through */
default:
appctx->st2 = STAT_ST_FIN;
return 1;
goto done;
}
done:
thread_release();
return 1;
full:
thread_release();
si_rx_room_blk(si);
return 0;
}
static void cli_release_show_sess(struct appctx *appctx)