Keep a master copy of the parameter block, to which all changes are applied,

and which is copied to the shared parameter block every time a parameter
changes as well as immediately before forking off a child.  This prevents a
hypothetical compromised child from changing the parent's idea of run-time
parameters (which would, for example, allow it to trick the the parent into
starting a new, hypothetically exploitable child with the attacker's choice
of uid / gid).

While I'm here, correct the use of the "volatile" qualifier - it is the
parmeter block itself which can change unpredictably, not the pointer.

git-svn-id: svn+ssh://projects.linpro.no/svn/varnish/trunk@1484 d4fa192b-c00b-0410-8231-f00ffab90ce4

diff --git a/varnish-cache/bin/varnishd/heritage.h b/varnish-cache/bin/varnishd/heritage.h
index a4cbf1a51d50aee015aa44cc8515a33aa59504fd..a89cdff42020c6c2119892360a92be4b0e28b53a 100644 (file)
--- a/varnish-cache/bin/varnishd/heritage.h
+++ b/varnish-cache/bin/varnishd/heritage.h
@@ -121,7 +121,7 @@ struct params {
        unsigned                ping_interval;
 };
 
-extern volatile struct params *params;
+extern struct params * volatile params;
 extern struct heritage heritage;
 
 void child_main(void);

diff --git a/varnish-cache/bin/varnishd/mgt.h b/varnish-cache/bin/varnishd/mgt.h
index 24b0f5284c34e59d91f16ee45c756dcb2ebcb717..f1a275bddee81063f28d7a4d4e9cf03eb5c0b194 100644 (file)
--- a/varnish-cache/bin/varnishd/mgt.h
+++ b/varnish-cache/bin/varnishd/mgt.h
@@ -52,6 +52,7 @@ void mgt_cli_stop_child(void);
 int mgt_cli_telnet(const char *T_arg);
 
 /* mgt_param.c */
+void MCF_ParamSync(void);
 void MCF_ParamInit(struct cli *);
 void MCF_ParamSet(struct cli *, const char *param, const char *val);
 

diff --git a/varnish-cache/bin/varnishd/mgt_child.c b/varnish-cache/bin/varnishd/mgt_child.c
index 216f22656268b8a358ea333dc2559be41ec3e0a9..dd09d482559d529067d610643d45d0fa346ee07a 100644 (file)
--- a/varnish-cache/bin/varnishd/mgt_child.c
+++ b/varnish-cache/bin/varnishd/mgt_child.c
@@ -173,6 +173,7 @@ start_child(void)
        AZ(pipe(&heritage.fds[0]));
        AZ(pipe(&heritage.fds[2]));
        AZ(pipe(child_fds));
+       MCF_ParamSync();
        i = fork();
        if (i < 0)
                errx(1, "Could not fork child");

diff --git a/varnish-cache/bin/varnishd/mgt_param.c b/varnish-cache/bin/varnishd/mgt_param.c
index fda73cd0e1de8063d40f9b0efd21dc182e98a5ab..1a8b230a77e44d7b66e7c13d7d1700ba1172b2d4 100644 (file)
--- a/varnish-cache/bin/varnishd/mgt_param.c
+++ b/varnish-cache/bin/varnishd/mgt_param.c
@@ -59,6 +59,8 @@ struct parspec {
        const char      *units;
 };
 
+static struct params master;
+
 /*--------------------------------------------------------------------*/
 
 static void
@@ -150,26 +152,26 @@ tweak_user(struct cli *cli, struct parspec *par, const char *arg)
                        cli_result(cli, CLIS_PARAM);
                        return;
                }
-               if (params->user)
-                       free(params->user);
-               params->user = strdup(pw->pw_name);
-               AN(params->user);
-               params->uid = pw->pw_uid;
+               if (master.user)
+                       free(master.user);
+               master.user = strdup(pw->pw_name);
+               AN(master.user);
+               master.uid = pw->pw_uid;
 
                /* set group to user's primary group */
-               if (params->group)
-                       free(params->group);
+               if (master.group)
+                       free(master.group);
                if ((gr = getgrgid(pw->pw_gid)) != NULL &&
                    (gr = getgrnam(gr->gr_name)) != NULL &&
                    gr->gr_gid == pw->pw_gid) {
-                       params->group = strdup(gr->gr_name);
-                       AN(params->group);
+                       master.group = strdup(gr->gr_name);
+                       AN(master.group);
                }
-               params->gid = pw->pw_gid;
-       } else if (params->user) {
-               cli_out(cli, "%s (%d)", params->user, (int)params->uid);
+               master.gid = pw->pw_gid;
+       } else if (master.user) {
+               cli_out(cli, "%s (%d)", master.user, (int)master.uid);
        } else {
-               cli_out(cli, "%d", (int)params->uid);
+               cli_out(cli, "%d", (int)master.uid);
        }
 }
 
@@ -187,15 +189,15 @@ tweak_group(struct cli *cli, struct parspec *par, const char *arg)
                        cli_result(cli, CLIS_PARAM);
                        return;
                }
-               if (params->group)
-                       free(params->group);
-               params->group = strdup(gr->gr_name);
-               AN(params->group);
-               params->gid = gr->gr_gid;
-       } else if (params->group) {
-               cli_out(cli, "%s (%d)", params->group, (int)params->gid);
+               if (master.group)
+                       free(master.group);
+               master.group = strdup(gr->gr_name);
+               AN(master.group);
+               master.gid = gr->gr_gid;
+       } else if (master.group) {
+               cli_out(cli, "%s (%d)", master.group, (int)master.gid);
        } else {
-               cli_out(cli, "%d", (int)params->gid);
+               cli_out(cli, "%d", (int)master.gid);
        }
 }
 
@@ -206,7 +208,7 @@ tweak_default_ttl(struct cli *cli, struct parspec *par, const char *arg)
 {
 
        (void)par;
-       tweak_generic_uint(cli, &params->default_ttl, arg, 0, UINT_MAX);
+       tweak_generic_uint(cli, &master.default_ttl, arg, 0, UINT_MAX);
 }
 
 /*--------------------------------------------------------------------*/
@@ -216,7 +218,7 @@ tweak_thread_pools(struct cli *cli, struct parspec *par, const char *arg)
 {
 
        (void)par;
-       tweak_generic_uint(cli, &params->wthread_pools, arg,
+       tweak_generic_uint(cli, &master.wthread_pools, arg,
            1, UINT_MAX);
 }
 
@@ -228,8 +230,8 @@ tweak_thread_pool_min(struct cli *cli, struct parspec *par, const char *arg)
 {
 
        (void)par;
-       tweak_generic_uint(cli, &params->wthread_min, arg,
-           0, params->wthread_max);
+       tweak_generic_uint(cli, &master.wthread_min, arg,
+           0, master.wthread_max);
 }
 
 /*--------------------------------------------------------------------*/
@@ -239,8 +241,8 @@ tweak_thread_pool_max(struct cli *cli, struct parspec *par, const char *arg)
 {
 
        (void)par;
-       tweak_generic_uint(cli, &params->wthread_max, arg,
-           params->wthread_min, UINT_MAX);
+       tweak_generic_uint(cli, &master.wthread_max, arg,
+           master.wthread_min, UINT_MAX);
 }
 
 /*--------------------------------------------------------------------*/
@@ -250,7 +252,7 @@ tweak_thread_pool_timeout(struct cli *cli, struct parspec *par, const char *arg)
 {
 
        (void)par;
-       tweak_generic_timeout(cli, &params->wthread_timeout, arg);
+       tweak_generic_timeout(cli, &master.wthread_timeout, arg);
 }
 
 /*--------------------------------------------------------------------*/
@@ -260,7 +262,7 @@ tweak_overflow_max(struct cli *cli, struct parspec *par, const char *arg)
 {
 
        (void)par;
-       tweak_generic_uint(cli, &params->overflow_max, arg, 0, UINT_MAX);
+       tweak_generic_uint(cli, &master.overflow_max, arg, 0, UINT_MAX);
 }
 
 /*--------------------------------------------------------------------*/
@@ -270,7 +272,7 @@ tweak_http_workspace(struct cli *cli, struct parspec *par, const char *arg)
 {
 
        (void)par;
-       tweak_generic_uint(cli, &params->mem_workspace, arg,
+       tweak_generic_uint(cli, &master.mem_workspace, arg,
            1024, UINT_MAX);
 }
 
@@ -280,7 +282,7 @@ static void
 tweak_sess_timeout(struct cli *cli, struct parspec *par, const char *arg)
 {
        (void)par;
-       tweak_generic_timeout(cli, &params->sess_timeout, arg);
+       tweak_generic_timeout(cli, &master.sess_timeout, arg);
 }
 
 /*--------------------------------------------------------------------*/
@@ -289,7 +291,7 @@ static void
 tweak_pipe_timeout(struct cli *cli, struct parspec *par, const char *arg)
 {
        (void)par;
-       tweak_generic_timeout(cli, &params->pipe_timeout, arg);
+       tweak_generic_timeout(cli, &master.pipe_timeout, arg);
 }
 
 /*--------------------------------------------------------------------*/
@@ -298,7 +300,7 @@ static void
 tweak_send_timeout(struct cli *cli, struct parspec *par, const char *arg)
 {
        (void)par;
-       tweak_generic_timeout(cli, &params->send_timeout, arg);
+       tweak_generic_timeout(cli, &master.send_timeout, arg);
 }
 
 /*--------------------------------------------------------------------*/
@@ -308,7 +310,7 @@ tweak_auto_restart(struct cli *cli, struct parspec *par, const char *arg)
 {
 
        (void)par;
-       tweak_generic_bool(cli, &params->auto_restart, arg);
+       tweak_generic_bool(cli, &master.auto_restart, arg);
 }
 
 /*--------------------------------------------------------------------*/
@@ -318,7 +320,7 @@ tweak_fetch_chunksize(struct cli *cli, struct parspec *par, const char *arg)
 {
 
        (void)par;
-       tweak_generic_uint(cli, &params->fetch_chunksize, arg,
+       tweak_generic_uint(cli, &master.fetch_chunksize, arg,
            4, UINT_MAX / 1024);
 }
 
@@ -330,7 +332,7 @@ tweak_sendfile_threshold(struct cli *cli, struct parspec *par, const char *arg)
 {
 
        (void)par;
-       tweak_generic_uint(cli, &params->sendfile_threshold, arg, 0, UINT_MAX);
+       tweak_generic_uint(cli, &master.sendfile_threshold, arg, 0, UINT_MAX);
 }
 #endif /* HAVE_SENDFILE */
 
@@ -340,7 +342,7 @@ static void
 tweak_vcl_trace(struct cli *cli, struct parspec *par, const char *arg)
 {
        (void)par;
-       tweak_generic_bool(cli, &params->vcl_trace, arg);
+       tweak_generic_bool(cli, &master.vcl_trace, arg);
 }
 
 /*--------------------------------------------------------------------*/
@@ -369,9 +371,9 @@ tweak_listen_address(struct cli *cli, struct parspec *par, const char *arg)
        if (arg == NULL) {
                /* Quote the string if we have more than one socket */
                if (heritage.nsocks > 1)
-                       cli_out(cli, "\"%s\"", params->listen_address);
+                       cli_out(cli, "\"%s\"", master.listen_address);
                else
-                       cli_out(cli, "%s", params->listen_address);
+                       cli_out(cli, "%s", master.listen_address);
                return;
        }
 
@@ -422,9 +424,9 @@ tweak_listen_address(struct cli *cli, struct parspec *par, const char *arg)
                return;
        }
 
-       free(params->listen_address);
-       params->listen_address = strdup(arg);
-       AN(params->listen_address);
+       free(master.listen_address);
+       master.listen_address = strdup(arg);
+       AN(master.listen_address);
 
        clean_listen_sock_head(&heritage.socks);
        heritage.nsocks = 0;
@@ -443,7 +445,7 @@ static void
 tweak_listen_depth(struct cli *cli, struct parspec *par, const char *arg)
 {
        (void)par;
-       tweak_generic_uint(cli, &params->listen_depth, arg, 0, UINT_MAX);
+       tweak_generic_uint(cli, &master.listen_depth, arg, 0, UINT_MAX);
 }
 
 /*--------------------------------------------------------------------*/
@@ -452,7 +454,7 @@ static void
 tweak_srcaddr_hash(struct cli *cli, struct parspec *par, const char *arg)
 {
        (void)par;
-       tweak_generic_uint(cli, &params->srcaddr_hash, arg, 63, UINT_MAX);
+       tweak_generic_uint(cli, &master.srcaddr_hash, arg, 63, UINT_MAX);
 }
 
 /*--------------------------------------------------------------------*/
@@ -461,7 +463,7 @@ static void
 tweak_srcaddr_ttl(struct cli *cli, struct parspec *par, const char *arg)
 {
        (void)par;
-       tweak_generic_uint(cli, &params->srcaddr_ttl, arg, 0, UINT_MAX);
+       tweak_generic_uint(cli, &master.srcaddr_ttl, arg, 0, UINT_MAX);
 }
 
 /*--------------------------------------------------------------------*/
@@ -470,7 +472,7 @@ static void
 tweak_backend_http11(struct cli *cli, struct parspec *par, const char *arg)
 {
        (void)par;
-       tweak_generic_bool(cli, &params->backend_http11, arg);
+       tweak_generic_bool(cli, &master.backend_http11, arg);
 }
 
 /*--------------------------------------------------------------------*/
@@ -479,7 +481,7 @@ static void
 tweak_client_http11(struct cli *cli, struct parspec *par, const char *arg)
 {
        (void)par;
-       tweak_generic_bool(cli, &params->client_http11, arg);
+       tweak_generic_bool(cli, &master.client_http11, arg);
 }
 
 /*--------------------------------------------------------------------*/
@@ -488,7 +490,7 @@ static void
 tweak_ping_interval(struct cli *cli, struct parspec *par, const char *arg)
 {
        (void)par;
-       tweak_generic_uint(cli, &params->ping_interval, arg, 0, UINT_MAX);
+       tweak_generic_uint(cli, &master.ping_interval, arg, 0, UINT_MAX);
 }
 
 /*--------------------------------------------------------------------*/
@@ -732,6 +734,15 @@ mcf_param_show(struct cli *cli, char **av, void *priv)
 
 /*--------------------------------------------------------------------*/
 
+void
+MCF_ParamSync(void)
+{
+       if (params != &master)
+               *params = master;
+}
+
+/*--------------------------------------------------------------------*/
+
 void
 MCF_ParamSet(struct cli *cli, const char *param, const char *val)
 {
@@ -745,6 +756,7 @@ MCF_ParamSet(struct cli *cli, const char *param, const char *val)
        }
        cli_result(cli, CLIS_PARAM);
        cli_out(cli, "Unknown paramter \"%s\".", param);
+       MCF_ParamSync();
 }
 
 
@@ -771,4 +783,5 @@ MCF_ParamInit(struct cli *cli)
                if (cli->result != CLIS_OK)
                        return;
        }
+       params = &master;
 }

diff --git a/varnish-cache/bin/varnishd/varnishd.c b/varnish-cache/bin/varnishd/varnishd.c
index 3fbacc12376a6b7e8e8b469a9ca23673153ca452..1a0e2f10dfef57d2bcc265d247124c6068afe9ed 100644 (file)
--- a/varnish-cache/bin/varnishd/varnishd.c
+++ b/varnish-cache/bin/varnishd/varnishd.c
@@ -66,7 +66,7 @@
 #endif
 
 struct heritage heritage;
-volatile struct params *params;
+struct params * volatile params;
 
 /*--------------------------------------------------------------------*/
 
@@ -407,7 +407,6 @@ main(int argc, char *argv[])
        const char *T_arg = NULL;
        unsigned C_flag = 0;
        char *p;
-       struct params param;
        struct cli cli[1];
        struct pidfh *pfh = NULL;
 
@@ -419,23 +418,7 @@ main(int argc, char *argv[])
        XXXAN(cli[0].sb);
        cli[0].result = CLIS_OK;
 
-       /*
-        * Set up a temporary param block until VSL_MgtInit() can
-        * replace with shmem backed structure version.
-        *
-        * XXX: I wonder if it would be smarter to inform the child process
-        * XXX: about param changes via CLI rather than keeping the param
-        * XXX: block in shared memory.  It would give us the advantage
-        * XXX: of having the CLI thread be able to take action on the
-        * XXX: change.
-        * XXX: For now live with the harmless flexelint warning this causes:
-        * XXX: varnishd.c 393 Info 789: Assigning address of auto variable
-        * XXX:    'param' to static
-        */
-
        TAILQ_INIT(&heritage.socks);
-       memset(&param, 0, sizeof param);
-       params = &param;
        mgt_vcc_init();
 
        MCF_ParamInit(cli);