From: Willy Tarreau <w@1wt.eu>
Date: Thu, 5 May 2022 16:29:25 +0000 (+0200)
Subject: BUG/MEDIUM: cli: make "show cli sockets" really yield
X-Git-Tag: v2.6-dev9~83
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=241a006d798b4305a64746f8da0b9fdeade533a5;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: cli: make "show cli sockets" really yield

This command was introduced in 1.8 with commit eceddf722 ("MEDIUM: cli:
'show cli sockets' list the CLI sockets") but its yielding doesn't work.
Each time it enters, it restarts from the last bind_conf but enumerates
all listening sockets again, thus it loops forever. The risk that it
happens in field is low but it easily triggers on port ranges after
400-500 sockets depending on the length of their addresses:

  global
     stats socket /tmp/sock1 level admin
     stats socket 192.168.8.176:30000-31000 level operator

  $ socat /tmp/sock1 - <<< "show cli sockets"
  (...)
  ipv4@192.168.8.176:30426 operator all
  ipv4@192.168.8.176:30427 operator all
  ipv4@192.168.8.176:30428 operator all
  ipv4@192.168.8.176:30000 operator all
  ipv4@192.168.8.176:30001 operator all
  ipv4@192.168.8.176:30002 operator all
  ^C

This patch adds the minimally needed restart point for the listener so
that it can easily be backported. Some more cleanup is needed though.
---

diff --git a/src/cli.c b/src/cli.c
index 7f6b58b4ef..014665242e 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -1530,11 +1530,11 @@ static int cli_io_handler_show_activity(struct appctx *appctx)
 
 /*
  * CLI IO handler for `show cli sockets`.
- * Uses ctx.cli.p0 to store the restart pointer.
+ * Uses ctx.cli.p0 to store the bind_conf pointer, and cli.p1 for the listener.
  */
 static int cli_io_handler_show_cli_sock(struct appctx *appctx)
 {
-	struct bind_conf *bind_conf;
+	struct bind_conf *bind_conf = appctx->ctx.cli.p0;
 	struct conn_stream *cs = appctx->owner;
 
 	chunk_reset(&trash);
@@ -1551,23 +1551,16 @@ static int cli_io_handler_show_cli_sock(struct appctx *appctx)
 
 		case STAT_ST_LIST:
 			if (global.cli_fe) {
-				list_for_each_entry(bind_conf, &global.cli_fe->conf.bind, by_fe) {
-					struct listener *l;
-
-					/*
-					 * get the latest dumped node in appctx->ctx.cli.p0
-					 * if the current node is the first of the list
-					 */
+				if (!bind_conf)
+					bind_conf = LIST_ELEM(global.cli_fe->conf.bind.n, typeof(bind_conf), by_fe);
 
-					if (appctx->ctx.cli.p0  &&
-					    &bind_conf->by_fe == (&global.cli_fe->conf.bind)->n) {
-						/* change the current node to the latest dumped and continue the loop */
-						bind_conf = LIST_ELEM(appctx->ctx.cli.p0, typeof(bind_conf), by_fe);
-						continue;
-					}
+				list_for_each_entry_from(bind_conf, &global.cli_fe->conf.bind, by_fe) {
+					struct listener *l = appctx->ctx.cli.p1;
 
-					list_for_each_entry(l, &bind_conf->listeners, by_bind) {
+					if (!l)
+						l = LIST_ELEM(bind_conf->listeners.n, typeof(l), by_bind);
 
+					list_for_each_entry_from(l, &bind_conf->listeners, by_bind) {
 						char addr[46];
 						char port[6];
 
@@ -1605,11 +1598,13 @@ static int cli_io_handler_show_cli_sock(struct appctx *appctx)
 						chunk_appendf(&trash, "all\n");
 
 						if (ci_putchk(cs_ic(cs), &trash) == -1) {
+							/* buffer full, we must yield */
+							appctx->ctx.cli.p0 = bind_conf;
+							appctx->ctx.cli.p1 = l;
 							cs_rx_room_blk(cs);
 							return 0;
 						}
 					}
-					appctx->ctx.cli.p0 = &bind_conf->by_fe; /* store the latest list node dumped */
 				}
 			}
 			/* fall through */