From: Willy Tarreau <w@1wt.eu>
Date: Fri, 30 Nov 2007 10:11:02 +0000 (+0100)
Subject: [MEDIUM] secure the calling conditions of ->set_server_status_{up,down}
X-Git-Tag: v1.3.14~31
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0ebe106ef1337a62b226a954d95225f0432c7963;p=thirdparty%2Fhaproxy.git

[MEDIUM] secure the calling conditions of ->set_server_status_{up,down}

It's not always obvious for the callers of set_server_status_{up,down}
whether the new state really is up or down. Some flags as well as the
effective weight have to be considered. Let's ensure that those functions
perform the necessary check themselves so that if the state transition
cannot be performed, at least everything is updated as required.
---

diff --git a/src/backend.c b/src/backend.c
index cc12f7ad62..1c544749f0 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -128,16 +128,19 @@ static void map_set_server_status_down(struct server *srv)
 	    srv->eweight == srv->prev_eweight)
 		return;
 
+	if (srv_is_usable(srv->state, srv->eweight))
+		goto out_update_state;
+
 	/* FIXME: could be optimized since we know what changed */
 	recount_servers(p);
 	update_backend_weight(p);
+	p->lbprm.map.state |= PR_MAP_RECALC;
+ out_update_state:
 	srv->prev_state = srv->state;
 	srv->prev_eweight = srv->eweight;
-	p->lbprm.map.state |= PR_MAP_RECALC;
-
 }
 
-/* this function updates the map according to server <srv>'s new state */
+/* This function updates the map according to server <srv>'s new state */
 static void map_set_server_status_up(struct server *srv)
 {
 	struct proxy *p = srv->proxy;
@@ -146,12 +149,16 @@ static void map_set_server_status_up(struct server *srv)
 	    srv->eweight == srv->prev_eweight)
 		return;
 
+	if (!srv_is_usable(srv->state, srv->eweight))
+		goto out_update_state;
+
 	/* FIXME: could be optimized since we know what changed */
 	recount_servers(p);
 	update_backend_weight(p);
+	p->lbprm.map.state |= PR_MAP_RECALC;
+ out_update_state:
 	srv->prev_state = srv->state;
 	srv->prev_eweight = srv->eweight;
-	p->lbprm.map.state |= PR_MAP_RECALC;
 }
 
 /* This function recomputes the server map for proxy px. It relies on
@@ -285,8 +292,9 @@ void init_server_map(struct proxy *p)
 
 /* This function updates the server trees according to server <srv>'s new
  * state. It should be called when server <srv>'s status changes to down.
- * It is not important whether the server was already down or not. However,
- * it is mandatory that the new state be down.
+ * It is not important whether the server was already down or not. It is not
+ * important either that the new state is completely down (the caller may not
+ * know all the variables of a server's state).
  */
 static void fwrr_set_server_status_down(struct server *srv)
 {
@@ -297,6 +305,9 @@ static void fwrr_set_server_status_down(struct server *srv)
 	    srv->eweight == srv->prev_eweight)
 		return;
 
+	if (srv_is_usable(srv->state, srv->eweight))
+		goto out_update_state;
+
 	if (!srv_is_usable(srv->prev_state, srv->prev_eweight))
 		/* server was already down */
 		goto out_update_backend;
@@ -331,15 +342,16 @@ static void fwrr_set_server_status_down(struct server *srv)
 out_update_backend:
 	/* check/update tot_used, tot_weight */
 	update_backend_weight(p);
+ out_update_state:
 	srv->prev_state = srv->state;
 	srv->prev_eweight = srv->eweight;
-
 }
 
 /* This function updates the server trees according to server <srv>'s new
  * state. It should be called when server <srv>'s status changes to up.
- * It is not important whether the server was already down or not. However,
- * it is mandatory that the new state be up. This function will not change
+ * It is not important whether the server was already down or not. It is not
+ * important either that the new state is completely UP (the caller may not
+ * know all the variables of a server's state). This function will not change
  * the weight of a server which was already up.
  */
 static void fwrr_set_server_status_up(struct server *srv)
@@ -351,6 +363,9 @@ static void fwrr_set_server_status_up(struct server *srv)
 	    srv->eweight == srv->prev_eweight)
 		return;
 
+	if (!srv_is_usable(srv->state, srv->eweight))
+		goto out_update_state;
+
 	if (srv_is_usable(srv->prev_state, srv->prev_eweight))
 		/* server was already up */
 		goto out_update_backend;
@@ -386,6 +401,7 @@ static void fwrr_set_server_status_up(struct server *srv)
 out_update_backend:
 	/* check/update tot_used, tot_weight */
 	update_backend_weight(p);
+ out_update_state:
 	srv->prev_state = srv->state;
 	srv->prev_eweight = srv->eweight;
 }