From a41d1bc642641e36d620138f93f484921d361bce Mon Sep 17 00:00:00 2001 From: rmacklem Date: Tue, 18 Dec 2012 00:25:48 +0000 Subject: Piete.Brooks at cl.cam.ac.uk reported via email a crash which was caused by use of an invalid kgss_gssd_handle during an upcall to the gssd daemon when it has exited. This patch seems to avoid the crashes by holding a reference count on the kgss_gssd_handle until the upcall is done. It also adds a new mutex kgss_gssd_lock used to make manipulation of kgss_gssd_handle SMP safe. Tested by: Illias A. Marinos, Herbert Poeckl Reviewed by: jhb MFC after: 2 weeks --- sys/kgssapi/gss_accept_sec_context.c | 11 +++++++++-- sys/kgssapi/gss_acquire_cred.c | 11 ++++++++--- sys/kgssapi/gss_canonicalize_name.c | 11 ++++++++--- sys/kgssapi/gss_delete_sec_context.c | 13 ++++++++++--- sys/kgssapi/gss_display_status.c | 11 ++++++++--- sys/kgssapi/gss_export_name.c | 11 ++++++++--- sys/kgssapi/gss_impl.c | 37 ++++++++++++++++++++++++++++++++---- sys/kgssapi/gss_import_name.c | 9 +++++++-- sys/kgssapi/gss_init_sec_context.c | 9 +++++++-- sys/kgssapi/gss_pname_to_uid.c | 24 +++++++++++++++-------- sys/kgssapi/gss_release_cred.c | 14 +++++++++++--- sys/kgssapi/gss_release_name.c | 13 ++++++++++--- sys/kgssapi/gss_set_cred_option.c | 9 +++++++-- sys/kgssapi/gssapi_impl.h | 2 ++ 14 files changed, 144 insertions(+), 41 deletions(-) (limited to 'sys') diff --git a/sys/kgssapi/gss_accept_sec_context.c b/sys/kgssapi/gss_accept_sec_context.c index 59f2803..3758a4a 100644 --- a/sys/kgssapi/gss_accept_sec_context.c +++ b/sys/kgssapi/gss_accept_sec_context.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -58,9 +60,13 @@ OM_uint32 gss_accept_sec_context(OM_uint32 *minor_status, gss_ctx_id_t ctx = *context_handle; gss_name_t name; gss_cred_id_t cred; + CLIENT *cl; - if (!kgss_gssd_handle) + cl = kgss_gssd_client(); + if (cl == NULL) { + *minor_status = 0; return (GSS_S_FAILURE); + } if (ctx) args.ctx = ctx->handle; @@ -74,7 +80,8 @@ OM_uint32 gss_accept_sec_context(OM_uint32 *minor_status, args.input_chan_bindings = input_chan_bindings; bzero(&res, sizeof(res)); - stat = gssd_accept_sec_context_1(&args, &res, kgss_gssd_handle); + stat = gssd_accept_sec_context_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); diff --git a/sys/kgssapi/gss_acquire_cred.c b/sys/kgssapi/gss_acquire_cred.c index e5fe821..4a5f115 100644 --- a/sys/kgssapi/gss_acquire_cred.c +++ b/sys/kgssapi/gss_acquire_cred.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -55,8 +57,11 @@ gss_acquire_cred(OM_uint32 *minor_status, enum clnt_stat stat; gss_cred_id_t cred; int i; + CLIENT *cl; - if (!kgss_gssd_handle) + *minor_status = 0; + cl = kgss_gssd_client(); + if (cl == NULL) return (GSS_S_FAILURE); args.uid = curthread->td_ucred->cr_uid; @@ -69,7 +74,8 @@ gss_acquire_cred(OM_uint32 *minor_status, args.cred_usage = cred_usage; bzero(&res, sizeof(res)); - stat = gssd_acquire_cred_1(&args, &res, kgss_gssd_handle); + stat = gssd_acquire_cred_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); @@ -80,7 +86,6 @@ gss_acquire_cred(OM_uint32 *minor_status, return (res.major_status); } - *minor_status = 0; cred = malloc(sizeof(struct _gss_cred_id_t), M_GSSAPI, M_WAITOK); cred->handle = res.output_cred; *output_cred_handle = cred; diff --git a/sys/kgssapi/gss_canonicalize_name.c b/sys/kgssapi/gss_canonicalize_name.c index bea3dd8..940f64a 100644 --- a/sys/kgssapi/gss_canonicalize_name.c +++ b/sys/kgssapi/gss_canonicalize_name.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -48,15 +50,19 @@ gss_canonicalize_name(OM_uint32 *minor_status, struct canonicalize_name_args args; enum clnt_stat stat; gss_name_t name; + CLIENT *cl; - if (!kgss_gssd_handle) + *minor_status = 0; + cl = kgss_gssd_client(); + if (cl == NULL) return (GSS_S_FAILURE); args.input_name = input_name->handle; args.mech_type = mech_type; bzero(&res, sizeof(res)); - stat = gssd_canonicalize_name_1(&args, &res, kgss_gssd_handle); + stat = gssd_canonicalize_name_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); @@ -69,7 +75,6 @@ gss_canonicalize_name(OM_uint32 *minor_status, name = malloc(sizeof(struct _gss_name_t), M_GSSAPI, M_WAITOK); name->handle = res.output_name; - *minor_status = 0; *output_name = name; return (GSS_S_COMPLETE); diff --git a/sys/kgssapi/gss_delete_sec_context.c b/sys/kgssapi/gss_delete_sec_context.c index e1582a2..8689927 100644 --- a/sys/kgssapi/gss_delete_sec_context.c +++ b/sys/kgssapi/gss_delete_sec_context.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -46,6 +48,9 @@ gss_delete_sec_context(OM_uint32 *minor_status, gss_ctx_id_t *context_handle, struct delete_sec_context_args args; enum clnt_stat stat; gss_ctx_id_t ctx; + CLIENT *cl; + + *minor_status = 0; if (!kgss_gssd_handle) return (GSS_S_FAILURE); @@ -60,9 +65,13 @@ gss_delete_sec_context(OM_uint32 *minor_status, gss_ctx_id_t *context_handle, */ if (ctx->handle) { args.ctx = ctx->handle; + cl = kgss_gssd_client(); + if (cl == NULL) + return (GSS_S_FAILURE); bzero(&res, sizeof(res)); - stat = gssd_delete_sec_context_1(&args, &res, kgss_gssd_handle); + stat = gssd_delete_sec_context_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); @@ -85,7 +94,5 @@ gss_delete_sec_context(OM_uint32 *minor_status, gss_ctx_id_t *context_handle, } } - *minor_status = 0; - return (GSS_S_COMPLETE); } diff --git a/sys/kgssapi/gss_display_status.c b/sys/kgssapi/gss_display_status.c index 0b5b79d..e03a376 100644 --- a/sys/kgssapi/gss_display_status.c +++ b/sys/kgssapi/gss_display_status.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -49,8 +51,11 @@ gss_display_status(OM_uint32 *minor_status, struct display_status_res res; struct display_status_args args; enum clnt_stat stat; + CLIENT *cl; - if (!kgss_gssd_handle) + *minor_status = 0; + cl = kgss_gssd_client(); + if (cl == NULL) return (GSS_S_FAILURE); args.status_value = status_value; @@ -59,7 +64,8 @@ gss_display_status(OM_uint32 *minor_status, args.message_context = *message_context; bzero(&res, sizeof(res)); - stat = gssd_display_status_1(&args, &res, kgss_gssd_handle); + stat = gssd_display_status_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); @@ -70,7 +76,6 @@ gss_display_status(OM_uint32 *minor_status, return (res.major_status); } - *minor_status = 0; *message_context = res.message_context; kgss_copy_buffer(&res.status_string, status_string); xdr_free((xdrproc_t) xdr_display_status_res, &res); diff --git a/sys/kgssapi/gss_export_name.c b/sys/kgssapi/gss_export_name.c index 63c1e8a..23c3ed8 100644 --- a/sys/kgssapi/gss_export_name.c +++ b/sys/kgssapi/gss_export_name.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -45,14 +47,18 @@ gss_export_name(OM_uint32 *minor_status, gss_name_t input_name, struct export_name_res res; struct export_name_args args; enum clnt_stat stat; + CLIENT *cl; - if (!kgss_gssd_handle) + *minor_status = 0; + cl = kgss_gssd_client(); + if (cl == NULL) return (GSS_S_FAILURE); args.input_name = input_name->handle; bzero(&res, sizeof(res)); - stat = gssd_export_name_1(&args, &res, kgss_gssd_handle); + stat = gssd_export_name_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); @@ -63,7 +69,6 @@ gss_export_name(OM_uint32 *minor_status, gss_name_t input_name, return (res.major_status); } - *minor_status = 0; kgss_copy_buffer(&res.exported_name, exported_name); xdr_free((xdrproc_t) xdr_export_name_res, &res); diff --git a/sys/kgssapi/gss_impl.c b/sys/kgssapi/gss_impl.c index d66b4a9..365a876 100644 --- a/sys/kgssapi/gss_impl.c +++ b/sys/kgssapi/gss_impl.c @@ -31,8 +31,10 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include +#include #include #include #include @@ -59,6 +61,7 @@ static bool_t gssd_syscall_registered = FALSE; struct kgss_mech_list kgss_mechs; CLIENT *kgss_gssd_handle; +struct mtx kgss_gssd_lock; static void kgss_init(void *dummy) @@ -92,14 +95,12 @@ sys_gssd_syscall(struct thread *td, struct gssd_syscall_args *uap) struct netconfig *nconf; char path[MAXPATHLEN]; int error; + CLIENT *cl, *oldcl; error = priv_check(td, PRIV_NFS_DAEMON); if (error) return (error); - if (kgss_gssd_handle) - CLNT_DESTROY(kgss_gssd_handle); - error = copyinstr(uap->path, path, sizeof(path), NULL); if (error) return (error); @@ -109,10 +110,20 @@ sys_gssd_syscall(struct thread *td, struct gssd_syscall_args *uap) sun.sun_len = SUN_LEN(&sun); nconf = getnetconfigent("local"); - kgss_gssd_handle = clnt_reconnect_create(nconf, + cl = clnt_reconnect_create(nconf, (struct sockaddr *) &sun, GSSD, GSSDVERS, RPC_MAXDATASIZE, RPC_MAXDATASIZE); + mtx_lock(&kgss_gssd_lock); + oldcl = kgss_gssd_handle; + kgss_gssd_handle = cl; + mtx_unlock(&kgss_gssd_lock); + + if (oldcl != NULL) { + CLNT_CLOSE(oldcl); + CLNT_RELEASE(oldcl); + } + return (0); } @@ -249,6 +260,23 @@ kgss_copy_buffer(const gss_buffer_t from, gss_buffer_t to) } /* + * Acquire the kgss_gssd_handle and return it with a reference count, + * if it is available. + */ +CLIENT * +kgss_gssd_client(void) +{ + CLIENT *cl; + + mtx_lock(&kgss_gssd_lock); + cl = kgss_gssd_handle; + if (cl != NULL) + CLNT_ACQUIRE(cl); + mtx_unlock(&kgss_gssd_lock); + return (cl); +} + +/* * Kernel module glue */ static int @@ -280,6 +308,7 @@ kgssapi_modevent(module_t mod, int type, void *data) rpc_gss_get_principal_name; rpc_gss_entries.rpc_gss_svc_max_data_length = rpc_gss_svc_max_data_length; + mtx_init(&kgss_gssd_lock, "kgss_gssd_lock", NULL, MTX_DEF); break; case MOD_UNLOAD: /* diff --git a/sys/kgssapi/gss_import_name.c b/sys/kgssapi/gss_import_name.c index c8019c5..d3a2240 100644 --- a/sys/kgssapi/gss_import_name.c +++ b/sys/kgssapi/gss_import_name.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -48,18 +50,21 @@ gss_import_name(OM_uint32 *minor_status, struct import_name_args args; enum clnt_stat stat; gss_name_t name; + CLIENT *cl; *minor_status = 0; *output_name = GSS_C_NO_NAME; - if (!kgss_gssd_handle) + cl = kgss_gssd_client(); + if (cl == NULL) return (GSS_S_FAILURE); args.input_name_buffer = *input_name_buffer; args.input_name_type = input_name_type; bzero(&res, sizeof(res)); - stat = gssd_import_name_1(&args, &res, kgss_gssd_handle); + stat = gssd_import_name_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); diff --git a/sys/kgssapi/gss_init_sec_context.c b/sys/kgssapi/gss_init_sec_context.c index 0b7cee3..64999e4 100644 --- a/sys/kgssapi/gss_init_sec_context.c +++ b/sys/kgssapi/gss_init_sec_context.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -60,10 +62,12 @@ gss_init_sec_context(OM_uint32 * minor_status, struct init_sec_context_args args; enum clnt_stat stat; gss_ctx_id_t ctx = *context_handle; + CLIENT *cl; *minor_status = 0; - if (!kgss_gssd_handle) + cl = kgss_gssd_client(); + if (cl == NULL) return (GSS_S_FAILURE); args.uid = curthread->td_ucred->cr_uid; @@ -88,7 +92,8 @@ gss_init_sec_context(OM_uint32 * minor_status, } bzero(&res, sizeof(res)); - stat = gssd_init_sec_context_1(&args, &res, kgss_gssd_handle); + stat = gssd_init_sec_context_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); diff --git a/sys/kgssapi/gss_pname_to_uid.c b/sys/kgssapi/gss_pname_to_uid.c index b83fd73..45ead36 100644 --- a/sys/kgssapi/gss_pname_to_uid.c +++ b/sys/kgssapi/gss_pname_to_uid.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -45,20 +47,23 @@ gss_pname_to_uid(OM_uint32 *minor_status, const gss_name_t pname, struct pname_to_uid_res res; struct pname_to_uid_args args; enum clnt_stat stat; + CLIENT *cl; *minor_status = 0; - if (!kgss_gssd_handle) - return (GSS_S_FAILURE); - if (pname == GSS_C_NO_NAME) return (GSS_S_BAD_NAME); + cl = kgss_gssd_client(); + if (cl == NULL) + return (GSS_S_FAILURE); + args.pname = pname->handle; args.mech = mech; bzero(&res, sizeof(res)); - stat = gssd_pname_to_uid_1(&args, &res, kgss_gssd_handle); + stat = gssd_pname_to_uid_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); @@ -83,20 +88,23 @@ gss_pname_to_unix_cred(OM_uint32 *minor_status, const gss_name_t pname, struct pname_to_uid_args args; enum clnt_stat stat; int i, n; + CLIENT *cl; *minor_status = 0; - if (!kgss_gssd_handle) - return (GSS_S_FAILURE); - if (pname == GSS_C_NO_NAME) return (GSS_S_BAD_NAME); + cl = kgss_gssd_client(); + if (cl == NULL) + return (GSS_S_FAILURE); + args.pname = pname->handle; args.mech = mech; bzero(&res, sizeof(res)); - stat = gssd_pname_to_uid_1(&args, &res, kgss_gssd_handle); + stat = gssd_pname_to_uid_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); diff --git a/sys/kgssapi/gss_release_cred.c b/sys/kgssapi/gss_release_cred.c index 6c68496..cad36fb 100644 --- a/sys/kgssapi/gss_release_cred.c +++ b/sys/kgssapi/gss_release_cred.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -44,13 +46,21 @@ gss_release_cred(OM_uint32 *minor_status, gss_cred_id_t *cred_handle) struct release_cred_res res; struct release_cred_args args; enum clnt_stat stat; + CLIENT *cl; + + *minor_status = 0; if (!kgss_gssd_handle) return (GSS_S_FAILURE); if (*cred_handle) { args.cred = (*cred_handle)->handle; - stat = gssd_release_cred_1(&args, &res, kgss_gssd_handle); + + cl = kgss_gssd_client(); + if (cl == NULL) + return (GSS_S_FAILURE); + stat = gssd_release_cred_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); @@ -63,7 +73,5 @@ gss_release_cred(OM_uint32 *minor_status, gss_cred_id_t *cred_handle) return (res.major_status); } - *minor_status = 0; - return (GSS_S_COMPLETE); } diff --git a/sys/kgssapi/gss_release_name.c b/sys/kgssapi/gss_release_name.c index 6f27e74..abda878 100644 --- a/sys/kgssapi/gss_release_name.c +++ b/sys/kgssapi/gss_release_name.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -45,6 +47,9 @@ gss_release_name(OM_uint32 *minor_status, gss_name_t *input_name) struct release_name_args args; enum clnt_stat stat; gss_name_t name; + CLIENT *cl; + + *minor_status = 0; if (!kgss_gssd_handle) return (GSS_S_FAILURE); @@ -53,7 +58,11 @@ gss_release_name(OM_uint32 *minor_status, gss_name_t *input_name) name = *input_name; args.input_name = name->handle; - stat = gssd_release_name_1(&args, &res, kgss_gssd_handle); + cl = kgss_gssd_client(); + if (cl == NULL) + return (GSS_S_FAILURE); + stat = gssd_release_name_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; return (GSS_S_FAILURE); @@ -68,7 +77,5 @@ gss_release_name(OM_uint32 *minor_status, gss_name_t *input_name) } } - *minor_status = 0; - return (GSS_S_COMPLETE); } diff --git a/sys/kgssapi/gss_set_cred_option.c b/sys/kgssapi/gss_set_cred_option.c index ce781af..d3b9a46 100644 --- a/sys/kgssapi/gss_set_cred_option.c +++ b/sys/kgssapi/gss_set_cred_option.c @@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include @@ -47,10 +49,12 @@ gss_set_cred_option(OM_uint32 *minor_status, struct set_cred_option_res res; struct set_cred_option_args args; enum clnt_stat stat; + CLIENT *cl; *minor_status = 0; - if (!kgss_gssd_handle) + cl = kgss_gssd_client(); + if (cl == NULL) return (GSS_S_FAILURE); if (cred) @@ -61,7 +65,8 @@ gss_set_cred_option(OM_uint32 *minor_status, args.option_value = *option_value; bzero(&res, sizeof(res)); - stat = gssd_set_cred_option_1(&args, &res, kgss_gssd_handle); + stat = gssd_set_cred_option_1(&args, &res, cl); + CLNT_RELEASE(cl); if (stat != RPC_SUCCESS) { *minor_status = stat; diff --git a/sys/kgssapi/gssapi_impl.h b/sys/kgssapi/gssapi_impl.h index 629b80b..c143255 100644 --- a/sys/kgssapi/gssapi_impl.h +++ b/sys/kgssapi/gssapi_impl.h @@ -53,8 +53,10 @@ struct kgss_mech { LIST_HEAD(kgss_mech_list, kgss_mech); extern CLIENT *kgss_gssd_handle; +extern struct mtx kgss_gssd_lock; extern struct kgss_mech_list kgss_mechs; +CLIENT *kgss_gssd_client(void); int kgss_oid_equal(const gss_OID oid1, const gss_OID oid2); extern void kgss_install_mech(gss_OID mech_type, const char *name, struct kobj_class *cls); -- cgit v1.1