[master] 1a75354bc Move the initialization of stevedores to the worker process, right before privileges are dropped.

Poul-Henning Kamp phk at FreeBSD.org
Wed Jun 1 09:14:05 UTC 2022


commit 1a75354bcc18b0c8c18670d54eb0720d7691ffe4
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Wed Jun 1 09:13:20 2022 +0000

    Move the initialization of stevedores to the worker process, right before privileges are dropped.

diff --git a/bin/varnishd/common/heritage.h b/bin/varnishd/common/heritage.h
index af312b265..7ef4b04ba 100644
--- a/bin/varnishd/common/heritage.h
+++ b/bin/varnishd/common/heritage.h
@@ -105,7 +105,6 @@ void MCH_Fd_Inherit(int fd, const char *what);
 
 #define ARGV_ERR(...)						\
 	do {							\
-		ASSERT_MGT();					\
 		fprintf(stderr, "Error: " __VA_ARGS__);		\
 		fprintf(stderr, "(-? gives usage)\n");		\
 		exit(2);					\
diff --git a/bin/varnishd/mgt/mgt.h b/bin/varnishd/mgt/mgt.h
index 2ca6a8594..4d54c86c0 100644
--- a/bin/varnishd/mgt/mgt.h
+++ b/bin/varnishd/mgt/mgt.h
@@ -220,6 +220,7 @@ char **MGT_NamedArg(const char *, const char **, const char *);
 extern const char *mgt_stv_h2_rxbuf;
 void STV_Config(const char *spec);
 void STV_Config_Transient(void);
+void STV_Init(void);
 
 /* mgt_vcc.c */
 void mgt_DumpBuiltin(void);
diff --git a/bin/varnishd/mgt/mgt_child.c b/bin/varnishd/mgt/mgt_child.c
index b4ebfba41..da4308c81 100644
--- a/bin/varnishd/mgt/mgt_child.c
+++ b/bin/varnishd/mgt/mgt_child.c
@@ -380,6 +380,8 @@ mgt_launch_child(struct cli *cli)
 
 		vext_load();
 
+		STV_Init();
+
 		VJ_subproc(JAIL_SUBPROC_WORKER);
 
 		heritage.proc_vsmw = VSMW_New(heritage.vsm_fd, 0640, "_.index");
diff --git a/bin/varnishd/storage/mgt_stevedore.c b/bin/varnishd/storage/mgt_stevedore.c
index c593c4d9d..e646d2669 100644
--- a/bin/varnishd/storage/mgt_stevedore.c
+++ b/bin/varnishd/storage/mgt_stevedore.c
@@ -46,7 +46,12 @@
 
 #include "storage/storage.h"
 
-static VTAILQ_HEAD(, stevedore) stevedores =
+VTAILQ_HEAD(stevedore_head, stevedore);
+
+static struct stevedore_head pre_stevedores =
+    VTAILQ_HEAD_INITIALIZER(pre_stevedores);
+
+static struct stevedore_head stevedores =
     VTAILQ_HEAD_INITIALIZER(stevedores);
 
 /* Name of transient storage */
@@ -66,8 +71,10 @@ STV__iter(struct stevedore ** const pp)
 	CHECK_OBJ_ORNULL(*pp, STEVEDORE_MAGIC);
 	if (*pp != NULL)
 		*pp = VTAILQ_NEXT(*pp, list);
-	else
+	else if (!VTAILQ_EMPTY(&stevedores))
 		*pp = VTAILQ_FIRST(&stevedores);
+	else
+		*pp = VTAILQ_FIRST(&pre_stevedores);
 	return (*pp != NULL);
 }
 
@@ -166,34 +173,12 @@ static const struct choice STV_choice[] = {
 	{ NULL,		NULL }
 };
 
-static void
-stv_check_ident(const char *spec, const char *ident)
-{
-	struct stevedore *stv;
-	unsigned found = 0;
-
-	if (!strcmp(ident, TRANSIENT_STORAGE))
-		found = (stv_transient != NULL);
-	else {
-		STV_Foreach(stv)
-			if (!strcmp(stv->ident, ident)) {
-				found = 1;
-				break;
-			}
-	}
-
-	if (found)
-		ARGV_ERR("(-s %s) '%s' is already defined\n", spec, ident);
-}
-
 void
 STV_Config(const char *spec)
 {
 	char **av, buf[8];
 	const char *ident;
 	struct stevedore *stv;
-	const struct stevedore *stv2;
-	int ac;
 	static unsigned seq = 0;
 
 	av = MGT_NamedArg(spec, &ident, "-s");
@@ -202,47 +187,24 @@ STV_Config(const char *spec)
 	if (av[1] == NULL)
 		ARGV_ERR("-s argument lacks strategy {malloc, file, ...}\n");
 
-	for (ac = 0; av[ac + 2] != NULL; ac++)
-		continue;
-
-	stv2 = MGT_Pick(STV_choice, av[1], "storage");
-	AN(stv2);
-
 	/* Append strategy to ident string */
 	VSB_printf(vident, ",-s%s", av[1]);
 
-	av += 2;
+	if (ident == NULL) {
+		bprintf(buf, "s%u", seq++);
+		ident = strdup(buf);
+	}
+
+	VTAILQ_FOREACH(stv, &pre_stevedores, list)
+		if (!strcmp(stv->ident, ident))
+			ARGV_ERR("(-s %s) '%s' is already defined\n", spec, ident);
 
-	CHECK_OBJ_NOTNULL(stv2, STEVEDORE_MAGIC);
 	ALLOC_OBJ(stv, STEVEDORE_MAGIC);
 	AN(stv);
-
-	*stv = *stv2;
-	AN(stv->name);
-
-	if (ident) {
-		stv->ident = ident;
-	} else {
-		bprintf(buf, "s%u", seq++);
-		stv->ident = strdup(buf);
-	}
-	AN(stv->ident);
-	stv_check_ident(spec, stv->ident);
-
-	if (stv->init != NULL)
-		stv->init(stv, ac, av);
-	else if (ac != 0)
-		ARGV_ERR("(-s %s) too many arguments\n", stv->name);
-
-	AN(stv->allocobj);
-	AN(stv->methods);
-
-	if (!strcmp(stv->ident, TRANSIENT_STORAGE)) {
-		AZ(stv_transient);
-		stv_transient = stv;
-	} else
-		VTAILQ_INSERT_TAIL(&stevedores, stv, list);
-	/* NB: Do not free av, stevedore gets to keep it */
+	stv->av = av;
+	stv->ident = ident;
+	stv->name = av[1];
+	VTAILQ_INSERT_TAIL(&pre_stevedores, stv, list);
 }
 
 /*--------------------------------------------------------------------*/
@@ -250,11 +212,67 @@ STV_Config(const char *spec)
 void
 STV_Config_Transient(void)
 {
+	struct stevedore *stv;
 	ASSERT_MGT();
 
 	VCLS_AddFunc(mgt_cls, MCF_AUTH, cli_stv);
-	if (stv_transient == NULL)
-		STV_Config(TRANSIENT_STORAGE "=default");
+	STV_Foreach(stv) {
+		if (!strcmp(stv->ident, TRANSIENT_STORAGE))
+			return;
+	}
+	STV_Config(TRANSIENT_STORAGE "=default");
+}
+
+/*--------------------------------------------------------------------
+ * Initialize configured stevedores in the worker process
+ */
+
+void
+STV_Init(void)
+{
+	char **av;
+	const char *ident;
+	struct stevedore *stv;
+	const struct stevedore *stv2;
+	int ac;
+
+	while (!VTAILQ_EMPTY(&pre_stevedores)) {
+		stv = VTAILQ_FIRST(&pre_stevedores);
+		VTAILQ_REMOVE(&pre_stevedores, stv, list);
+		CHECK_OBJ_NOTNULL(stv, STEVEDORE_MAGIC);
+		AN(stv->av);
+		av = stv->av;
+		AN(stv->ident);
+		ident = stv->ident;
+
+		for (ac = 0; av[ac + 2] != NULL; ac++)
+			continue;
+
+		stv2 = MGT_Pick(STV_choice, av[1], "storage");
+		CHECK_OBJ_NOTNULL(stv2, STEVEDORE_MAGIC);
+		*stv = *stv2;
+		AN(stv->name);
+
+		av += 2;
+
+		stv->ident = ident;
+		stv->av = av;
+
+		if (stv->init != NULL)
+			stv->init(stv, ac, av);
+		else if (ac != 0)
+			ARGV_ERR("(-s %s) too many arguments\n", stv->name);
+
+		AN(stv->allocobj);
+		AN(stv->methods);
+
+		if (!strcmp(stv->ident, TRANSIENT_STORAGE)) {
+			AZ(stv_transient);
+			stv_transient = stv;
+		} else
+			VTAILQ_INSERT_TAIL(&stevedores, stv, list);
+		/* NB: Do not free av, stevedore gets to keep it */
+	}
 	AN(stv_transient);
 	VTAILQ_INSERT_TAIL(&stevedores, stv_transient, list);
 }
diff --git a/bin/varnishd/storage/mgt_storage_persistent.c b/bin/varnishd/storage/mgt_storage_persistent.c
index fdd4419c8..e70c8414f 100644
--- a/bin/varnishd/storage/mgt_storage_persistent.c
+++ b/bin/varnishd/storage/mgt_storage_persistent.c
@@ -145,11 +145,8 @@ smp_mgt_init(struct stevedore *parent, int ac, char * const *av)
 	void *target;
 	int i, mmap_flags;
 
-	ASSERT_MGT();
-
 	AZ(av[ac]);
 
-
 #ifdef HAVE_SYS_PERSONALITY_H
 	i = personality(0xffffffff); /* Fetch old personality. */
 	if (!(i & ADDR_NO_RANDOMIZE)) {
diff --git a/bin/varnishd/storage/storage.h b/bin/varnishd/storage/storage.h
index b9017f579..882ab82b8 100644
--- a/bin/varnishd/storage/storage.h
+++ b/bin/varnishd/storage/storage.h
@@ -114,6 +114,7 @@ struct stevedore {
 	void				*priv;
 
 	VTAILQ_ENTRY(stevedore)	list;
+	char				**av;
 	const char			*ident;
 	const char			*vclname;
 };
diff --git a/bin/varnishd/storage/storage_malloc.c b/bin/varnishd/storage/storage_malloc.c
index fb7345271..2c3ce5b5b 100644
--- a/bin/varnishd/storage/storage_malloc.c
+++ b/bin/varnishd/storage/storage_malloc.c
@@ -178,7 +178,6 @@ sma_init(struct stevedore *parent, int ac, char * const *av)
 	uintmax_t u;
 	struct sma_sc *sc;
 
-	ASSERT_MGT();
 	ALLOC_OBJ(sc, SMA_SC_MAGIC);
 	AN(sc);
 	sc->sma_max = VRT_INTEGER_MAX;
diff --git a/bin/varnishd/storage/storage_persistent_subr.c b/bin/varnishd/storage/storage_persistent_subr.c
index 8a1ba8808..0506c57eb 100644
--- a/bin/varnishd/storage/storage_persistent_subr.c
+++ b/bin/varnishd/storage/storage_persistent_subr.c
@@ -279,8 +279,6 @@ smp_newsilo(struct smp_sc *sc)
 {
 	struct smp_ident	*si;
 
-	ASSERT_MGT();
-
 	/* Choose a new random number */
 	AZ(VRND_RandomCrypto(&sc->unique, sizeof sc->unique));
 
diff --git a/bin/varnishd/storage/storage_umem.c b/bin/varnishd/storage/storage_umem.c
index 2df9f345f..770efb7d7 100644
--- a/bin/varnishd/storage/storage_umem.c
+++ b/bin/varnishd/storage/storage_umem.c
@@ -297,7 +297,6 @@ smu_init(struct stevedore *parent, int ac, char * const *av)
 	uintmax_t u;
 	struct smu_sc *sc;
 
-	ASSERT_MGT();
 	ALLOC_OBJ(sc, SMU_SC_MAGIC);
 	AN(sc);
 	sc->smu_max = VRT_INTEGER_MAX;
diff --git a/bin/varnishtest/tests/p00000.vtc b/bin/varnishtest/tests/p00000.vtc
index f523ef82c..6956a7562 100644
--- a/bin/varnishtest/tests/p00000.vtc
+++ b/bin/varnishtest/tests/p00000.vtc
@@ -2,9 +2,14 @@ varnishtest "Test Basic persistence"
 
 feature persistent_storage
 
-shell -err -expect {-spersistent has been deprecated} {
-	varnishd -spersistent -f '' -n ${tmpdir}
-}
+process p1 -dump {
+	varnishd -spersistent -b localhost -n ${tmpdir} -a :0 -d 2>&1
+} -start -expect-exit 1
+
+process p1 -expect-text 0 0 {to launch}
+process p1 -write "start\n"
+process p1 -expect-text 0 0 {-spersistent has been deprecated}
+process p1 -wait
 
 server s1 {
 	rxreq


More information about the varnish-commit mailing list