From 7ba9f9467d95fa8d05bb04d36fd4c602e497f529 Mon Sep 17 00:00:00 2001
From: Russ Cox <rsc@swtch.com>
Date: Mon, 13 Jan 2020 11:41:37 -0500
Subject: lib9/fmt: avoid racy access to installed fmt formats

---
 src/lib9/fmt/fmt.c     | 35 ++++++++++++++++++++++-------------
 src/lib9/fmt/fmtdef.h  |  6 ++++--
 src/lib9/fmt/fmtlock.c | 14 ++++++++++++--
 src/lib9/fmtlock2.c    | 22 +++++++++++++++++-----
 4 files changed, 55 insertions(+), 22 deletions(-)

(limited to 'src/lib9')

diff --git a/src/lib9/fmt/fmt.c b/src/lib9/fmt/fmt.c
index 66093fd0..9c3f45d3 100644
--- a/src/lib9/fmt/fmt.c
+++ b/src/lib9/fmt/fmt.c
@@ -72,7 +72,7 @@ static Convfmt knownfmt[] = {
 int	(*fmtdoquote)(int);
 
 /*
- * __fmtlock() must be set
+ * __fmtwlock() must be set
  */
 static int
 __fmtinstall(int c, Fmts f)
@@ -106,34 +106,43 @@ fmtinstall(int c, int (*f)(Fmt*))
 {
 	int ret;
 
-	__fmtlock();
+	__fmtwlock();
 	ret = __fmtinstall(c, f);
-	__fmtunlock();
+	__fmtwunlock();
 	return ret;
 }
 
 static Fmts
 fmtfmt(int c)
 {
-	Convfmt *p, *ep;
+	Convfmt *p, *ep, *kp;
 
+	/* conflict-free check - common case */
+	__fmtrlock();
 	ep = &fmtalloc.fmt[fmtalloc.nfmt];
 	for(p=fmtalloc.fmt; p<ep; p++)
 		if(p->c == c){
-			while(p->fmt == nil)	/* loop until value is updated */
-				;
+			__fmtrunlock();
 			return p->fmt;
 		}
+	__fmtrunlock();
 
 	/* is this a predefined format char? */
-	__fmtlock();
-	for(p=knownfmt; p->c; p++)
-		if(p->c == c){
-			__fmtinstall(p->c, p->fmt);
-			__fmtunlock();
-			return p->fmt;
+	for(kp=knownfmt; kp->c; kp++){
+		if(kp->c == c){
+			__fmtwlock();
+			/* double-check fmtinstall didn't happen */
+			for(p=fmtalloc.fmt; p<ep; p++){
+				if(p->c == c){
+					__fmtwunlock();
+					return p->fmt;
+				}
+			}
+			__fmtinstall(kp->c, kp->fmt);
+			__fmtwunlock();
+			return kp->fmt;
 		}
-	__fmtunlock();
+	}
 
 	return __badfmt;
 }
diff --git a/src/lib9/fmt/fmtdef.h b/src/lib9/fmt/fmtdef.h
index 5c8eb2cb..d547184d 100644
--- a/src/lib9/fmt/fmtdef.h
+++ b/src/lib9/fmt/fmtdef.h
@@ -33,11 +33,13 @@ int          __fmtFdFlush(Fmt *f);
 int          __fmtcpy(Fmt *f, const void *vm, int n, int sz);
 void*        __fmtdispatch(Fmt *f, void *fmt, int isrunes);
 void *       __fmtflush(Fmt *f, void *t, int len);
-void         __fmtlock(void);
 int          __fmtpad(Fmt *f, int n);
 double       __fmtpow10(int n);
 int          __fmtrcpy(Fmt *f, const void *vm, int n);
-void         __fmtunlock(void);
+void         __fmtrlock(void);
+void         __fmtrunlock(void);
+void         __fmtwlock(void);
+void         __fmtwunlock(void);
 int          __ifmt(Fmt *f);
 int          __isInf(double d, int sign);
 int          __isNaN(double d);
diff --git a/src/lib9/fmt/fmtlock.c b/src/lib9/fmt/fmtlock.c
index cabe05f4..eb9cd845 100644
--- a/src/lib9/fmt/fmtlock.c
+++ b/src/lib9/fmt/fmtlock.c
@@ -5,11 +5,21 @@
 #include "fmtdef.h"
 
 void
-__fmtlock(void)
+__fmtrlock(void)
 {
 }
 
 void
-__fmtunlock(void)
+__fmtrunlock(void)
+{
+}
+
+void
+__fmtwlock(void)
+{
+}
+
+void
+__fmtwunlock(void)
 {
 }
diff --git a/src/lib9/fmtlock2.c b/src/lib9/fmtlock2.c
index d711e6d4..b755daa3 100644
--- a/src/lib9/fmtlock2.c
+++ b/src/lib9/fmtlock2.c
@@ -1,16 +1,28 @@
 #include <u.h>
 #include <libc.h>
 
-static Lock fmtlock;
+static RWLock fmtlock;
 
 void
-__fmtlock(void)
+__fmtrlock(void)
 {
-	lock(&fmtlock);
+	rlock(&fmtlock);
 }
 
 void
-__fmtunlock(void)
+__fmtrunlock(void)
 {
-	unlock(&fmtlock);
+	runlock(&fmtlock);
+}
+
+void
+__fmtwlock(void)
+{
+	wlock(&fmtlock);
+}
+
+void
+__fmtwunlock(void)
+{
+	wunlock(&fmtlock);
 }
-- 
cgit v1.2.3