From: Mark Stapp <mjs@voltanet.io>
Date: Sun, 17 Mar 2019 17:25:16 +0000 (-0400)
Subject: libs: fix race in privs changes
X-Git-Tag: 7.1_pulled~155^2
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=064e2f32807a549e777d829cd35e3bd941e95c06;p=mirror%2Ffrr.git

libs: fix race in privs changes

Use the privs struct mutex more strictly, to ensure that the
privs are at the level the caller expects when the apis
return.

Signed-off-by: Mark Stapp <mjs@voltanet.io>
---

diff --git a/lib/privs.c b/lib/privs.c
index 3ce8e0d57a..59f24afe4a 100644
--- a/lib/privs.c
+++ b/lib/privs.c
@@ -708,19 +708,19 @@ struct zebra_privs_t *_zprivs_raise(struct zebra_privs_t *privs,
 
 	/* If we're already elevated, just return */
 	pthread_mutex_lock(&(privs->mutex));
-	if (++privs->refcount > 1) {
-		pthread_mutex_unlock(&(privs->mutex));
-		return privs;
+	{
+		if (++(privs->refcount) == 1) {
+			errno = 0;
+			if (privs->change(ZPRIVS_RAISE)) {
+				zlog_err("%s: Failed to raise privileges (%s)",
+					 funcname, safe_strerror(errno));
+			}
+			errno = save_errno;
+			privs->raised_in_funcname = funcname;
+		}
 	}
 	pthread_mutex_unlock(&(privs->mutex));
 
-	errno = 0;
-	if (privs->change(ZPRIVS_RAISE)) {
-		zlog_err("%s: Failed to raise privileges (%s)",
-			 funcname, safe_strerror(errno));
-	}
-	errno = save_errno;
-	privs->raised_in_funcname = funcname;
 	return privs;
 }
 
@@ -733,19 +733,20 @@ void _zprivs_lower(struct zebra_privs_t **privs)
 
 	/* Don't lower privs if there's another caller */
 	pthread_mutex_lock(&(*privs)->mutex);
-	if (--((*privs)->refcount) > 0) {
-		pthread_mutex_unlock(&(*privs)->mutex);
-		return;
+	{
+		if (--((*privs)->refcount) == 0) {
+			errno = 0;
+			if ((*privs)->change(ZPRIVS_LOWER)) {
+				zlog_err("%s: Failed to lower privileges (%s)",
+					 (*privs)->raised_in_funcname,
+					 safe_strerror(errno));
+			}
+			errno = save_errno;
+			(*privs)->raised_in_funcname = NULL;
+		}
 	}
 	pthread_mutex_unlock(&(*privs)->mutex);
 
-	errno = 0;
-	if ((*privs)->change(ZPRIVS_LOWER)) {
-		zlog_err("%s: Failed to lower privileges (%s)",
-			 (*privs)->raised_in_funcname, safe_strerror(errno));
-	}
-	errno = save_errno;
-	(*privs)->raised_in_funcname = NULL;
 	*privs = NULL;
 }