d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix a race in the RPCSEC_GSS server code that caused crashes.

Browse Source 
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.

Tested by:	peter@ifm.liu.se
PR:		235582
MFC after:	2 weeks
This commit is contained in:
Rick Macklem 2019-04-02 23:51:08 +00:00
parent 154c6ffd71
commit b0e14530a0
1 changed files with 7 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
sys/rpc/rpcsec_gss/svc_rpcsec_gss.c
View File

@ -568,19 +568,13 @@ 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);
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 % svc_rpc_gss_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
@ -590,6 +584,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 % svc_rpc_gss_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);
}
@ -1287,7 +1287,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)) {