[4.1] 3f62309 Move the cli buffer to VSB (from stack)

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


commit 3f6230961612599f117537c1f5184cef2e9c1ca7
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
    
    Conflicts:
    	bin/varnishd/mgt/mgt_cli.c

diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index fa021d6..bcab2af 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -64,6 +64,8 @@ static const char	*secret_file;
 #define	MCF_NOAUTH	0	/* NB: zero disables here-documents */
 #define MCF_AUTH	16
 
+struct vsb		*cli_buf = NULL;
+
 /*--------------------------------------------------------------------*/
 
 static void
@@ -191,24 +193,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 9a272a3..3546b67 100644
--- a/include/tbl/params.h
+++ b/include/tbl/params.h
@@ -235,10 +235,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