From 06ec69c59f7b6a4d64d7950c5a440aabb81d5c70 Mon Sep 17 00:00:00 2001 From: rmacklem Date: Tue, 16 Apr 2019 02:48:04 +0000 Subject: [PATCH] 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. git-svn-id: svn://svn.freebsd.org/base/stable/10@346262 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- sys/rpc/rpcsec_gss/svc_rpcsec_gss.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c b/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c index 1d0794381..7cd105801 100644 --- a/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c +++ b/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c @@ -543,18 +543,17 @@ 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; 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 @@ -564,6 +563,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); } @@ -1261,7 +1266,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)) { -- 2.45.0