Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 16 Apr 2019 02:38:40 +0000 (UTC)
From:      Rick Macklem <rmacklem@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r346260 - stable/11/sys/rpc/rpcsec_gss
Message-ID:  <201904160238.x3G2ceiS062160@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: rmacklem
Date: Tue Apr 16 02:38:39 2019
New Revision: 346260
URL: https://svnweb.freebsd.org/changeset/base/346260

Log:
  MFC: r345818, r345828
  Fix a race in the RPCSEC_GSS server code that caused crashes.
  
  When a new client structure was allocated, it was added to the list
  so that it was visible to other threads before the expiry time was
  initialized, with only a single reference count.
  The caller would increment the reference count, but it was possible
  for another thread to decrement the reference count to zero and free
  the structure before the caller incremented the reference count.
  This could occur because the expiry time was still set to zero when
  the new client structure was inserted in the list and the list was
  unlocked.
  
  This patch fixes the race by initializing the reference count to two
  and initializing all fields, including the expiry time, before inserting
  it in the list.

Modified:
  stable/11/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c
==============================================================================
--- stable/11/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c	Tue Apr 16 02:28:35 2019	(r346259)
+++ stable/11/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c	Tue Apr 16 02:38:39 2019	(r346260)
@@ -559,19 +559,18 @@ svc_rpc_gss_create_client(void)
 
 	client = mem_alloc(sizeof(struct svc_rpc_gss_client));
 	memset(client, 0, sizeof(struct svc_rpc_gss_client));
-	refcount_init(&client->cl_refs, 1);
+
+	/*
+	 * Set the initial value of cl_refs to two.  One for the caller
+	 * and the other to hold onto the client structure until it expires.
+	 */
+	refcount_init(&client->cl_refs, 2);
 	sx_init(&client->cl_lock, "GSS-client");
 	getcredhostid(curthread->td_ucred, &hostid);
 	client->cl_id.ci_hostid = hostid;
 	getboottime(&boottime);
 	client->cl_id.ci_boottime = boottime.tv_sec;
 	client->cl_id.ci_id = svc_rpc_gss_next_clientid++;
-	list = &svc_rpc_gss_client_hash[client->cl_id.ci_id % CLIENT_HASH_SIZE];
-	sx_xlock(&svc_rpc_gss_lock);
-	TAILQ_INSERT_HEAD(list, client, cl_link);
-	TAILQ_INSERT_HEAD(&svc_rpc_gss_clients, client, cl_alllink);
-	svc_rpc_gss_client_count++;
-	sx_xunlock(&svc_rpc_gss_lock);
 
 	/*
 	 * Start the client off with a short expiration time. We will
@@ -581,6 +580,12 @@ svc_rpc_gss_create_client(void)
 	client->cl_locked = FALSE;
 	client->cl_expiration = time_uptime + 5*60;
 
+	list = &svc_rpc_gss_client_hash[client->cl_id.ci_id % CLIENT_HASH_SIZE];
+	sx_xlock(&svc_rpc_gss_lock);
+	TAILQ_INSERT_HEAD(list, client, cl_link);
+	TAILQ_INSERT_HEAD(&svc_rpc_gss_clients, client, cl_alllink);
+	svc_rpc_gss_client_count++;
+	sx_xunlock(&svc_rpc_gss_lock);
 	return (client);
 }
 
@@ -1278,7 +1283,6 @@ svc_rpc_gss(struct svc_req *rqst, struct rpc_msg *msg)
 			goto out;
 		}
 		client = svc_rpc_gss_create_client();
-		refcount_acquire(&client->cl_refs);
 	} else {
 		struct svc_rpc_gss_clientid *p;
 		if (gc.gc_handle.length != sizeof(*p)) {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201904160238.x3G2ceiS062160>