RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2024-12-14 23:44:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

MEDIUM: stream-int: remove dangerous interval checks for stream-int states

Browse Source 
The stream interface state checks involving ranges were replaced with
checks on a set of states, already revealing some issues. No issue was
fixed, all was replaced in a one-to-one mapping for easier control. Some
checks involving a strict difference were also replaced with fields to
be clearer. At this stage, the result must be strictly equivalent. A few
tests were also turned to their bit-field equivalent for better readability
or in preparation for upcoming changes.

The test performed in the SPOE filter was swapped so that the closed and
error states are evicted first and that the established vs conn state is
tested second.
This commit is contained in:
Willy Tarreau 2019-06-05 14:53:22 +02:00
parent bedcd698b3
commit 7ab22adbf7
4 changed files with 39 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
include/proto/stream_interface.h
View File

@ -184,7 +184,7 @@ static inline void si_release_endpoint(struct stream_interface *si)
cs_destroy(cs);
}
else if ((appctx = objt_appctx(si->end))) {
if (appctx->applet->release && si->state < SI_ST_DIS)
if (appctx->applet->release && !si_state_in(si->state, SI_SB_DIS|SI_SB_CLO))
appctx->applet->release(appctx);
appctx_free(appctx); /* we share the connection pool */
} else if ((conn = objt_conn(si->end))) {
@ -238,7 +238,7 @@ static inline void si_applet_release(struct stream_interface *si)
struct appctx *appctx;
appctx = objt_appctx(si->end);
if (appctx && appctx->applet->release && si->state < SI_ST_DIS)
if (appctx && appctx->applet->release && !si_state_in(si->state, SI_SB_DIS|SI_SB_CLO))
appctx->applet->release(appctx);
}
@ -448,13 +448,13 @@ static inline void si_must_kill_conn(struct stream_interface *si)
*/
static inline void si_chk_rcv(struct stream_interface *si)
{
if (si->flags & SI_FL_RXBLK_CONN && (si_opposite(si)->state >= SI_ST_EST))
if (si->flags & SI_FL_RXBLK_CONN && si_state_in(si_opposite(si)->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO))
si_rx_conn_rdy(si);
if (si_rx_blocked(si) || !si_rx_endp_ready(si))
return;
if (si->state != SI_ST_EST)
if (!si_state_in(si->state, SI_SB_EST))
return;
si->flags |= SI_FL_RX_WAIT_EP;
@ -472,7 +472,7 @@ static inline int si_sync_recv(struct stream_interface *si)
{
struct conn_stream *cs;
if (si->state != SI_ST_EST)
if (!si_state_in(si->state, SI_SB_EST))
return 0;
cs = objt_cs(si->end);

13
src/flt_spoe.c
View File

@ -1341,15 +1341,18 @@ spoe_handle_connect_appctx(struct appctx *appctx)
char *frame, *buf;
int ret;
if (si->state <= SI_ST_CON) {
if (si_state_in(si->state, SI_SB_CER|SI_SB_DIS|SI_SB_CLO)) {
/* closed */
SPOE_APPCTX(appctx)->status_code = SPOE_FRM_ERR_IO;
goto exit;
}
if (!si_state_in(si->state, SI_SB_EST)) {
/* not connected yet */
si_rx_endp_more(si);
task_wakeup(si_strm(si)->task, TASK_WOKEN_MSG);
goto stop;
}
if (si->state != SI_ST_EST) {
SPOE_APPCTX(appctx)->status_code = SPOE_FRM_ERR_IO;
goto exit;
}
if (appctx->st1 == SPOE_APPCTX_ERR_TOUT) {
SPOE_PRINTF(stderr, "%d.%06d [SPOE/%-15s] %s: appctx=%p"

28
src/stream.c
View File

@ -1138,8 +1138,8 @@ abort_connection:
static void sess_set_term_flags(struct stream *s)
{
if (!(s->flags & SF_FINST_MASK)) {
if (s->si[1].state < SI_ST_REQ) {
if (s->si[1].state == SI_ST_INI) {
/* anything before REQ in fact */
_HA_ATOMIC_ADD(&strm_fe(s)->fe_counters.failed_req, 1);
if (strm_li(s) && strm_li(s)->counters)
_HA_ATOMIC_ADD(&strm_li(s)->counters->failed_req, 1);
@ -1148,7 +1148,7 @@ static void sess_set_term_flags(struct stream *s)
}
else if (s->si[1].state == SI_ST_QUE)
s->flags |= SF_FINST_Q;
else if (s->si[1].state < SI_ST_EST)
else if (si_state_in(s->si[1].state, SI_SB_REQ|SI_SB_TAR|SI_SB_ASS|SI_SB_CON|SI_SB_CER))
s->flags |= SF_FINST_C;
else if (s->si[1].state == SI_ST_EST || s->si[1].prev_state == SI_ST_EST)
s->flags |= SF_FINST_D;
@ -1906,7 +1906,7 @@ redo:
*/
srv = objt_server(s->target);
if (unlikely(si_f->flags & SI_FL_ERR)) {
if (si_f->state == SI_ST_EST || si_f->state == SI_ST_DIS) {
if (si_state_in(si_f->state, SI_SB_EST|SI_SB_DIS)) {
si_shutr(si_f);
si_shutw(si_f);
si_report_error(si_f);
@ -1924,7 +1924,7 @@ redo:
}
if (unlikely(si_b->flags & SI_FL_ERR)) {
if (si_b->state == SI_ST_EST || si_b->state == SI_ST_DIS) {
if (si_state_in(si_b->state, SI_SB_EST|SI_SB_DIS)) {
si_shutr(si_b);
si_shutw(si_b);
si_report_error(si_b);
@ -1945,7 +1945,7 @@ redo:
/* note: maybe we should process connection errors here ? */
}
if (si_b->state == SI_ST_CON) {
if (si_state_in(si_b->state, SI_SB_CON)) {
/* we were trying to establish a connection on the server side,
* maybe it succeeded, maybe it failed, maybe we timed out, ...
*/
@ -2016,7 +2016,7 @@ redo:
s->pending_events & TASK_WOKEN_MSG) {
unsigned int flags = req->flags;
if (si_f->state >= SI_ST_EST) {
if (si_state_in(si_f->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO)) {
int max_loops = global.tune.maxpollevents;
unsigned int ana_list;
unsigned int ana_back;
@ -2117,7 +2117,7 @@ redo:
s->pending_events & TASK_WOKEN_MSG) {
unsigned int flags = res->flags;
if (si_b->state >= SI_ST_EST) {
if (si_state_in(si_b->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO)) {
int max_loops = global.tune.maxpollevents;
unsigned int ana_list;
unsigned int ana_back;
@ -2282,7 +2282,7 @@ redo:
*/
if (unlikely((!req->analysers || (req->analysers == AN_REQ_FLT_END && !(req->flags & CF_FLT_ANALYZE))) &&
!(req->flags & (CF_SHUTW|CF_SHUTR_NOW)) &&
(si_f->state >= SI_ST_EST) &&
(si_state_in(si_f->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO)) &&
(req->to_forward != CHN_INFINITE_FORWARD))) {
/* This buffer is freewheeling, there's no analyser
* attached to it. If any data are left in, we'll permit them to
@ -2401,8 +2401,7 @@ redo:
/* we may have a pending connection request, or a connection waiting
* for completion.
*/
if (si_b->state >= SI_ST_REQ && si_b->state < SI_ST_CON) {
if (si_state_in(si_b->state, SI_SB_REQ|SI_SB_QUE|SI_SB_TAR|SI_SB_ASS)) {
/* prune the request variables and swap to the response variables. */
if (s->vars_reqres.scope != SCOPE_RES) {
if (!LIST_ISEMPTY(&s->vars_reqres.head)) {
@ -2429,7 +2428,7 @@ redo:
/* Now we can add the server name to a header (if requested) */
/* check for HTTP mode and proxy server_name_hdr_name != NULL */
if ((si_b->state == SI_ST_CON || si_b->state == SI_ST_EST) &&
if (si_state_in(si_b->state, SI_SB_CON|SI_SB_EST) &&
(s->be->server_id_hdr_name != NULL) &&
(s->be->mode == PR_MODE_HTTP) &&
objt_server(s->target)) {
@ -2459,7 +2458,7 @@ redo:
*/
if (unlikely((!res->analysers || (res->analysers == AN_RES_FLT_END && !(res->flags & CF_FLT_ANALYZE))) &&
!(res->flags & (CF_SHUTW|CF_SHUTR_NOW)) &&
(si_b->state >= SI_ST_EST) &&
si_state_in(si_b->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO) &&
(res->to_forward != CHN_INFINITE_FORWARD))) {
/* This buffer is freewheeling, there's no analyser
* attached to it. If any data are left in, we'll permit them to
@ -2607,8 +2606,7 @@ redo:
}
}
if (likely((si_f->state != SI_ST_CLO) ||
(si_b->state > SI_ST_INI && si_b->state < SI_ST_CLO))) {
if (likely((si_f->state != SI_ST_CLO) || !si_state_in(si_b->state, SI_SB_INI|SI_SB_CLO))) {
enum si_state si_b_prev_state, si_f_prev_state;
si_f_prev_state = si_f->prev_state;

26
src/stream_interface.c
View File

@ -175,7 +175,7 @@ static void stream_int_shutr(struct stream_interface *si)
ic->flags |= CF_SHUTR;
ic->rex = TICK_ETERNITY;
if (si->state != SI_ST_EST && si->state != SI_ST_CON)
if (!si_state_in(si->state, SI_SB_CON|SI_SB_EST))
return;
if (si_oc(si)->flags & CF_SHUTW) {
@ -541,7 +541,7 @@ static void stream_int_notify(struct stream_interface *si)
/* wake the task up only when needed */
if (/* changes on the production side */
(ic->flags & (CF_READ_NULL|CF_READ_ERROR)) ||
(si->state != SI_ST_EST && si->state != SI_ST_CON) ||
!si_state_in(si->state, SI_SB_CON|SI_SB_EST) ||
(si->flags & SI_FL_ERR) ||
((ic->flags & CF_READ_PARTIAL) &&
((ic->flags & CF_EOI) || !ic->to_forward || sio->state != SI_ST_EST)) ||
@ -602,7 +602,7 @@ static int si_cs_process(struct conn_stream *cs)
task_wakeup(si_task(si), TASK_WOKEN_MSG);
}
if ((si->state < SI_ST_EST) &&
if (!si_state_in(si->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO) &&
(conn->flags & (CO_FL_CONNECTED | CO_FL_HANDSHAKE)) == CO_FL_CONNECTED) {
si->exp = TICK_ETERNITY;
oc->flags |= CF_WRITE_NULL;
@ -893,7 +893,7 @@ void si_update_both(struct stream_interface *si_f, struct stream_interface *si_b
/* back stream-int */
cs = objt_cs(si_b->end);
if (cs &&
(si_b->state == SI_ST_EST || si_b->state == SI_ST_CON) &&
si_state_in(si_b->state, SI_SB_CON|SI_SB_EST) &&
!(req->flags & CF_SHUTW) && /* Write not closed */
!channel_is_empty(req) &&
!(cs->flags & (CS_FL_ERROR|CS_FL_ERR_PENDING)) &&
@ -903,10 +903,10 @@ void si_update_both(struct stream_interface *si_f, struct stream_interface *si_b
}
/* let's recompute both sides states */
if (si_f->state == SI_ST_EST)
if (si_state_in(si_f->state, SI_SB_EST))
si_update(si_f);
if (si_b->state == SI_ST_EST)
if (si_state_in(si_b->state, SI_SB_EST))
si_update(si_b);
/* stream ints are processed outside of process_stream() and must be
@ -944,7 +944,7 @@ static void stream_int_shutr_conn(struct stream_interface *si)
ic->flags |= CF_SHUTR;
ic->rex = TICK_ETERNITY;
if (si->state != SI_ST_EST && si->state != SI_ST_CON)
if (!si_state_in(si->state, SI_SB_CON|SI_SB_EST))
return;
if (si->flags & SI_FL_KILL_CONN)
@ -1060,7 +1060,7 @@ static void stream_int_shutw_conn(struct stream_interface *si)
static void stream_int_chk_rcv_conn(struct stream_interface *si)
{
/* (re)start reading */
if (si->state == SI_ST_CON || si->state == SI_ST_EST)
if (si_state_in(si->state, SI_SB_CON|SI_SB_EST))
tasklet_wakeup(si->wait_event.task);
}
@ -1075,7 +1075,7 @@ static void stream_int_chk_snd_conn(struct stream_interface *si)
struct channel *oc = si_oc(si);
struct conn_stream *cs = __objt_cs(si->end);
if (unlikely((si->state != SI_ST_CON && si->state != SI_ST_EST) ||
if (unlikely(!si_state_in(si->state, SI_SB_CON|SI_SB_EST) ||
(oc->flags & CF_SHUTW)))
return;
@ -1106,7 +1106,7 @@ static void stream_int_chk_snd_conn(struct stream_interface *si)
*/
if (((oc->flags & (CF_SHUTW|CF_AUTO_CLOSE|CF_SHUTW_NOW)) ==
(CF_AUTO_CLOSE|CF_SHUTW_NOW)) &&
(si->state == SI_ST_EST)) {
si_state_in(si->state, SI_SB_EST)) {
si_shutw(si);
goto out_wakeup;
}
@ -1151,7 +1151,7 @@ static void stream_int_chk_snd_conn(struct stream_interface *si)
if (likely((oc->flags & (CF_WRITE_NULL|CF_WRITE_ERROR|CF_SHUTW)) ||
((oc->flags & CF_WAKE_WRITE) &&
((channel_is_empty(oc) && !oc->to_forward) ||
si->state != SI_ST_EST)))) {
!si_state_in(si->state, SI_SB_EST))))) {
out_wakeup:
if (!(si->flags & SI_FL_DONT_WAKE))
task_wakeup(si_task(si), TASK_WOKEN_IO);
@ -1471,7 +1471,7 @@ static void stream_int_read0(struct stream_interface *si)
ic->flags |= CF_SHUTR;
ic->rex = TICK_ETERNITY;
if (si->state != SI_ST_EST && si->state != SI_ST_CON)
if (!si_state_in(si->state, SI_SB_CON|SI_SB_EST))
return;
if (oc->flags & CF_SHUTW)
@ -1561,7 +1561,7 @@ static void stream_int_shutr_applet(struct stream_interface *si)
/* Note: on shutr, we don't call the applet */
if (si->state != SI_ST_EST && si->state != SI_ST_CON)
if (!si_state_in(si->state, SI_SB_CON|SI_SB_EST))
return;
if (si_oc(si)->flags & CF_SHUTW) {