shithub: riscv

Info  •  Files  •  Log  •  Branches

devfs: fix locking and ignore undocumented "fsdev:\n" configuration signature

The confstr was shared between readers so seprintconf() could
write concurrently to that buffer which is not safe.

This replaces the shared static confstr[Maxconf] buffer with a
pointer that is initially nil and a buffer that is alloced on
demand.

The new confstr pointer (and buffer) is now only updated while
wlock()ed from the new setconfstr() function.

This is now done by mconfig() / mdelctl() just before releasing
the wlock.

Now, rdconf() will check if confstr has been initialized, and
test for it again while wlock()ed; making sure the configuration
is read only once.

Also, rdconf() used to check for a undocumented "fsdev:\n" string
at the beginning of config data tho that was never documented.

This changes mconfig() to ignore that particular signature so
the example from the manpage will work as documented.

--- a/sys/src/9/port/devfs.c

+++ b/sys/src/9/port/devfs.c

@@ -117,12 +117,10 @@

 static char *disk;		/* default tree name used */

 static char *source;		/* default inner device used */

 static int sectorsz = Sectorsz;	/* default sector size */

-static char confstr[Maxconf];	/* textual configuration */

+static char *confstr;		/* textual configuration */

 static int debug;

-static char cfgstr[] = "fsdev:\n";

-

 static Qid tqid = {Qtop, 0, QTDIR};

 static Qid cqid = {Qctl, 0, 0};

@@ -180,6 +178,36 @@

 	return s;

 }

+static char*

+seprintconf(char *s, char *e)

+{

+	int	i, j;

+	Tree	*t;

+

+	*s = 0;

+	for(i = 0; i < ntrees; i++){

+		t = trees[i];

+		if(t != nil)

+			for(j = 0; j < t->nadevs; j++)

+				if(t->devs[j] != nil)

+					s = seprintdev(s, e, t->devs[j]);

+	}

+	return s;

+}

+

+/* called with lck w */

+static void

+setconfstr(void)

+{

+	char *s;

+

+	s = confstr;

+	if(s == nil)

+		s = smalloc(Maxconf);

+	seprintconf(s, s+Maxconf);

+	confstr = s;

+}

+

 static vlong

 mkpath(int tree, int devno)

 {

@@ -405,6 +433,8 @@

 			}

 		}

 	}

+	if(some)

+		setconfstr();

 	wunlock(&lck);

 	if(some == 0 && alltrees == 0)

 		error(Enonexist);

@@ -560,9 +590,13 @@

 	Tree	*t;

 	/* ignore comments & empty lines */

-	if (*a == '\0' || *a == '#' || *a == '\n')

+	if (n < 1 || *a == '\0' || *a == '#' || *a == '\n')

 		return;

+	/* ignore historical config signature */

+	if (n >= 6 && memcmp(a, "fsdev:", 6) == 0)

+		return;

+

 	dprint("mconfig\n");

 	size = 0;

 	start = 0;

@@ -722,8 +756,9 @@

 	}

 	setdsize(mp, ilen);

-	poperror();

+	setconfstr();

 	wunlock(&lck);

+	poperror();

 	free(idev);

 	free(ilen);

 	free(cb);

@@ -732,22 +767,31 @@

 static void

 rdconf(void)

 {

-	int mustrd;

 	char *c, *e, *p, *s;

 	Chan *cc;

-	static int configed;

+	int mustrd;

 	/* only read config file once */

-	if (configed)

+	if (confstr != nil)

 		return;

-	configed = 1;

-	dprint("rdconf\n");

+	wlock(&lck);

+	if (confstr != nil) {

+		wunlock(&lck);

+		return;	/* already done */

+	}

+

 	/* add the std "fs" tree */

-	trees[0] = &fstree;

-	ntrees++;

-	fstree.name = "fs";

+	if(ntrees == 0){

+		fstree.name = "fs";

+		trees[ntrees++] = &fstree;

+	}

+	setconfstr();

+	wunlock(&lck);

+

+	dprint("rdconf\n");

+

 	/* identify the config file */

 	s = getconf("fsconfig");

 	if (s == nil){

@@ -756,7 +800,9 @@

 	} else

 		mustrd = 1;

+	c = smalloc(Maxconf+1);

 	if(waserror()){

+		free(c);

 		if(!mustrd)

 			return;

 		nexterror();

@@ -768,23 +814,12 @@

 		cclose(cc);

 		nexterror();

 	}

-	devtab[cc->type]->read(cc, confstr, sizeof confstr, 0);

-	poperror();

+	devtab[cc->type]->read(cc, c, Maxconf, 0);

 	cclose(cc);

+	poperror();

-	/* validate, copy and erase config; mconfig will repopulate confstr */

-	if (strncmp(confstr, cfgstr, sizeof cfgstr - 1) != 0)

-		error("bad #k config, first line must be: 'fsdev:\\n'");

-

-	c = nil;

-	kstrdup(&c, confstr + sizeof cfgstr - 1);

-	if(waserror()){

-		free(c);

-		nexterror();

-	}

-	memset(confstr, 0, sizeof confstr);

 	/* process config copy one line at a time */

-	for (p = c; p != nil && *p != '\0'; p = e){

+	for (p = c; *p != '\0'; p = e){

 		e = strchr(p, '\n');

 		if (e == nil)

 			e = p + strlen(p);

@@ -792,10 +827,9 @@

 			e++;

 		mconfig(p, e - p);

 	}

-	poperror();

-	free(c);

-	poperror();	/* mustrd */

+	free(c);

+	poperror();	/* c */

 }

 static int

@@ -1138,23 +1172,6 @@

 	return res;

 }

-static char*

-seprintconf(char *s, char *e)

-{

-	int	i, j;

-	Tree	*t;

-

-	*s = 0;

-	for(i = 0; i < ntrees; i++){

-		t = trees[i];

-		if(t != nil)

-			for(j = 0; j < t->nadevs; j++)

-				if(t->devs[j] != nil)

-					s = seprintdev(s, e, t->devs[j]);

-	}

-	return s;

-}

-

 static long

 mread(Chan *c, void *a, long n, vlong off)

 {

@@ -1175,7 +1192,6 @@

 		goto Done;

 	}

 	if(c->qid.path == Qctl){

-		seprintconf(confstr, confstr + sizeof(confstr));

 		res = readstr((long)off, a, n, confstr);

 		goto Done;

 	}