From 8282b829a8e64811c1786f473e8679898ddfaa38 Mon Sep 17 00:00:00 2001 From: des Date: Fri, 1 Jun 2007 22:18:55 +0000 Subject: [PATCH] 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 --- varnish-cache/bin/varnishd/heritage.h | 2 +- varnish-cache/bin/varnishd/mgt.h | 1 + varnish-cache/bin/varnishd/mgt_child.c | 1 + varnish-cache/bin/varnishd/mgt_param.c | 109 ++++++++++++++----------- varnish-cache/bin/varnishd/varnishd.c | 19 +---- 5 files changed, 65 insertions(+), 67 deletions(-) diff --git a/varnish-cache/bin/varnishd/heritage.h b/varnish-cache/bin/varnishd/heritage.h index a4cbf1a5..a89cdff4 100644 --- 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 24b0f528..f1a275bd 100644 --- 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 216f2265..dd09d482 100644 --- 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 fda73cd0..1a8b230a 100644 --- 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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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, ¶ms->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 3fbacc12..1a0e2f10 100644 --- 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(¶m, 0, sizeof param); - params = ¶m; mgt_vcc_init(); MCF_ParamInit(cli); -- 2.39.5