[master] 3f5bc7f Move the cli buffer to VSB (from stack)

PÃ¥l Hermunn Johansen hermunn at varnish-software.com
Mon Sep 11 14:25:06 UTC 2017


commit 3f5bc7f3480b043a8480b7cd31c735f8fc843d16
Author: Pål Hermunn Johansen <hermunn at varnish-software.com>
Date:   Mon Sep 11 16:21:19 2017 +0200

    Move the cli buffer to VSB (from stack)
    
    If the cli buffer is allocated on the stack, the mgt process will
    crash hard for big values of this parameter. Instead we allocate on
    the heap.
    
    The new cli buffer an "auto" VSB. This means that the varnish
    parameter cli_buffer is ignored from now on, and the manual marks it
    as deprecated.
    
    Closes: #2382

diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index d7fa6a6..ff60b2d 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -67,6 +67,8 @@ static int		cli_i = -1, cli_o = -1;
 struct VCLS		*mgt_cls;
 static const char	*secret_file;
 
+struct vsb		*cli_buf = NULL;
+
 /*--------------------------------------------------------------------*/
 
 static void __match_proto__(cli_func_t)
@@ -177,24 +179,30 @@ mgt_cli_askchild(unsigned *status, char **resp, const char *fmt, ...)
 	int i, j;
 	va_list ap;
 	unsigned u;
-	char buf[mgt_param.cli_buffer], *p;
+
+	if (cli_buf == NULL) {
+		cli_buf = VSB_new_auto();
+		AN(cli_buf);
+	} else {
+		VSB_clear(cli_buf);
+	}
 
 	if (resp != NULL)
 		*resp = NULL;
 	if (status != NULL)
 		*status = 0;
-	if (cli_i < 0|| cli_o < 0) {
+	if (cli_i < 0 || cli_o < 0) {
 		if (status != NULL)
 			*status = CLIS_CANT;
 		return (CLIS_CANT);
 	}
 	va_start(ap, fmt);
-	vbprintf(buf, fmt, ap);
+	AZ(VSB_vprintf(cli_buf, fmt, ap));
 	va_end(ap);
-	p = strchr(buf, '\0');
-	assert(p != NULL && p > buf && p[-1] == '\n');
-	i = p - buf;
-	j = write(cli_o, buf, i);
+	AZ(VSB_finish(cli_buf));
+	i = VSB_len(cli_buf);
+	assert(i > 0 && VSB_data(cli_buf)[i - 1] == '\n');
+	j = write(cli_o, VSB_data(cli_buf), i);
 	if (j != i) {
 		if (status != NULL)
 			*status = CLIS_COMMS;
diff --git a/bin/varnishtest/tests/r02382.vtc b/bin/varnishtest/tests/r02382.vtc
new file mode 100644
index 0000000..7a398b3
--- /dev/null
+++ b/bin/varnishtest/tests/r02382.vtc
@@ -0,0 +1,21 @@
+varnishtest "Very very big cli_buffer"
+
+# If the cli_buffer is on the stack, setting it very big can crash varnish
+
+server s1 {
+       rxreq
+       expect req.url == "/1"
+       txresp
+} -start
+
+varnish v1 -arg "-p cli_buffer=32M" -vcl+backend {
+} -start
+
+varnish v1 -cliok "ping"
+
+delay .5
+
+client c1 {
+       txreq -url /1
+       rxresp
+} -run
diff --git a/include/tbl/params.h b/include/tbl/params.h
index 91cfd66..837c62f 100644
--- a/include/tbl/params.h
+++ b/include/tbl/params.h
@@ -294,10 +294,8 @@ PARAM(
 	/* units */	"bytes",
 	/* flags */	0,
 	/* s-text */
-	"Size of buffer for CLI command input.\n"
-	"You may need to increase this if you have big VCL files and use "
-	"the vcl.inline CLI command.\n"
-	"NB: Must be specified with -p to have effect.",
+	"DEPRECATED: This parameter is ignored.\n"
+	"Memory for the CLI command buffer is now dynamically allocated.",
 	/* l-text */	"",
 	/* func */	NULL
 )


More information about the varnish-commit mailing list