[lustre-devel] [PATCH 14/21] lustre: gss: fix ptlrpc_gss automatic loading

[James Simmons]

From: Sebastien Buisson <sbuisson at ddn.com>

ptlrpc_gss kernel module is automatically loaded when a GSS security
flavor is enforced. Loading success is recorded in a static variable
in the ptlrpc module, which prevents further reloading in case
ptlrpc_gss is unloaded while keeping ptlrpc loaded.
Get rid of this static variable as it is not required in order to
avoid calling request_module("ptlrpc_gss") when not needed. Indeed,
once loaded, the static array policies[] has an entry at the
SPTLRPC_POLICY_GSS index, indicating that the ptlrpc_gss module is
loaded.

WC-bug-id: https://jira.whamcloud.com/browse/LU-16888
Lustre-commit: b80d6defb7b018250 ("LU-16888 gss: fix ptlrpc_gss automatic loading")
Signed-off-by: Sebastien Buisson <sbuisson at ddn.com>
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/51264
Reviewed-by: Aurelien Degremont <adegremont at nvidia.com>
Reviewed-by: Andreas Dilger <adilger at whamcloud.com>
Reviewed-by: Oleg Drokin <green at whamcloud.com>
Signed-off-by: James Simmons <jsimmons at infradead.org>
---
 fs/lustre/ptlrpc/sec.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/fs/lustre/ptlrpc/sec.c b/fs/lustre/ptlrpc/sec.c
index 576e6480cda8..c9ad27617836 100644
--- a/fs/lustre/ptlrpc/sec.c
+++ b/fs/lustre/ptlrpc/sec.c
@@ -116,10 +116,9 @@ static
 struct ptlrpc_sec_policy *sptlrpc_wireflavor2policy(u32 flavor)
 {
 	static DEFINE_MUTEX(load_mutex);
-	static atomic_t loaded = ATOMIC_INIT(0);
 	struct ptlrpc_sec_policy *policy;
 	u16 number = SPTLRPC_FLVR_POLICY(flavor);
-	u16 flag = 0;
+	int rc;
 
 	if (number >= SPTLRPC_POLICY_MAX)
 		return NULL;
@@ -129,25 +128,26 @@ struct ptlrpc_sec_policy *sptlrpc_wireflavor2policy(u32 flavor)
 		policy = policies[number];
 		if (policy && !try_module_get(policy->sp_owner))
 			policy = NULL;
-		if (!policy)
-			flag = atomic_read(&loaded);
 		read_unlock(&policy_lock);
 
-		if (policy || flag != 0 ||
-		    number != SPTLRPC_POLICY_GSS)
+		if (policy || number != SPTLRPC_POLICY_GSS)
 			break;
 
-		/* try to load gss module, once */
+		/* try to load gss module, happens only if policy at index
+		 * SPTLRPC_POLICY_GSS is not already referenced in
+		 * global array policies[]
+		 */
 		mutex_lock(&load_mutex);
-		if (atomic_read(&loaded) == 0) {
-			if (request_module("ptlrpc_gss") == 0)
-				CDEBUG(D_SEC,
-				       "module ptlrpc_gss loaded on demand\n");
-			else
-				CERROR("Unable to load module ptlrpc_gss\n");
-
-			atomic_set(&loaded, 1);
-		}
+		/* The fact that request_module() returns 0 does not guarantee
+		 * the module has done its job. So we must check that the
+		 * requested policy is now available. This is done by checking
+		 * again for policies[number] in the loop.
+		 */
+		rc = request_module("ptlrpc_gss");
+		if (rc == 0)
+			CDEBUG(D_SEC, "module ptlrpc_gss loaded on demand\n");
+		else
+			CERROR("Unable to load module ptlrpc_gss: rc %d\n", rc);
 		mutex_unlock(&load_mutex);
 	}
 
-- 
2.39.3