BUG/MEDIUM: snapshot: take the proxy's lock while dumping errors
The proxy's lock it held while filling the error but not while dumping
it, so it's possible to dereference pointers being replaced, typically
server pointers. The risk is very low and unlikely but not inexistent.
Since "show errors" is rarely used in parallel, let's simply grab the
proxy's lock while dumping. Ideally we should use an R/W lock here but
it will not make any difference.
This patch must be backported to 1.8, but the code is in proto_http.c
there, though mostly similar.
diff --git a/src/proxy.c b/src/proxy.c
index 6447301..9306637 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -1995,11 +1995,8 @@
tm.tm_hour, tm.tm_min, tm.tm_sec, (int)(date.tv_usec/1000),
error_snapshot_id);
- if (ci_putchk(si_ic(si), &trash) == -1) {
- /* Socket buffer full. Let's try again later from the same point */
- si_applet_cant_put(si);
- return 0;
- }
+ if (ci_putchk(si_ic(si), &trash) == -1)
+ goto cant_send;
appctx->ctx.errors.px = proxies_list;
appctx->ctx.errors.bol = 0;
@@ -2012,6 +2009,8 @@
while (appctx->ctx.errors.px) {
struct error_snapshot *es;
+ HA_SPIN_LOCK(PROXY_LOCK, &appctx->ctx.errors.px->lock);
+
if ((appctx->ctx.errors.flag & 1) == 0) {
es = &appctx->ctx.errors.px->invalid_req;
if (appctx->ctx.errors.flag & 2) // skip req
@@ -2086,11 +2085,9 @@
chunk_appendf(&trash, " \n");
- if (ci_putchk(si_ic(si), &trash) == -1) {
- /* Socket buffer full. Let's try again later from the same point */
- si_applet_cant_put(si);
- return 0;
- }
+ if (ci_putchk(si_ic(si), &trash) == -1)
+ goto cant_send_unlock;
+
appctx->ctx.errors.ptr = 0;
appctx->ctx.errors.ev_id = es->ev_id;
}
@@ -2099,10 +2096,9 @@
/* the snapshot changed while we were dumping it */
chunk_appendf(&trash,
" WARNING! update detected on this snapshot, dump interrupted. Please re-check!\n");
- if (ci_putchk(si_ic(si), &trash) == -1) {
- si_applet_cant_put(si);
- return 0;
- }
+ if (ci_putchk(si_ic(si), &trash) == -1)
+ goto cant_send_unlock;
+
goto next;
}
@@ -2114,17 +2110,16 @@
newline = appctx->ctx.errors.bol;
newptr = dump_text_line(&trash, es->buf, global.tune.bufsize, es->buf_len, &newline, appctx->ctx.errors.ptr);
if (newptr == appctx->ctx.errors.ptr)
- return 0;
+ goto cant_send_unlock;
- if (ci_putchk(si_ic(si), &trash) == -1) {
- /* Socket buffer full. Let's try again later from the same point */
- si_applet_cant_put(si);
- return 0;
- }
+ if (ci_putchk(si_ic(si), &trash) == -1)
+ goto cant_send_unlock;
+
appctx->ctx.errors.ptr = newptr;
appctx->ctx.errors.bol = newline;
};
next:
+ HA_SPIN_UNLOCK(PROXY_LOCK, &appctx->ctx.errors.px->lock);
appctx->ctx.errors.bol = 0;
appctx->ctx.errors.ptr = -1;
appctx->ctx.errors.flag ^= 1;
@@ -2134,6 +2129,12 @@
/* dump complete */
return 1;
+
+ cant_send_unlock:
+ HA_SPIN_UNLOCK(PROXY_LOCK, &appctx->ctx.errors.px->lock);
+ cant_send:
+ si_applet_cant_put(si);
+ return 0;
}
/* register cli keywords */