* [PATCH nfs-utils] gssd: use mutex to protect decrement of refcount
@ 2021-05-21 4:54 NeilBrown
2021-05-23 18:28 ` Steve Dickson
0 siblings, 1 reply; 2+ messages in thread
From: NeilBrown @ 2021-05-21 4:54 UTC (permalink / raw
To: Steve Dickson; +Cc: Linux NFS Mailing list
The decrement of the "ple" refcount is not protected so it can race with
increments or decrements from other threads. An increment could be lost
and then the ple would be freed early, leading to memory corruption.
So use the mutex to protect decrements (increments are already
protected).
As gssd_destroy_krb5_principals() calls release_ple() while holding the
mutex, we need a "release_pte_locked()" which doesn't take the mutex.
Signed-off-by: NeilBrown <neilb@suse.de>
---
utils/gssd/krb5_util.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index 28b60ba307d0..51e0c6a2484b 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -169,18 +169,28 @@ static int gssd_get_single_krb5_cred(krb5_context context,
static int query_krb5_ccache(const char* cred_cache, char **ret_princname,
char **ret_realm);
-static void release_ple(krb5_context context, struct gssd_k5_kt_princ *ple)
+static void release_ple_locked(krb5_context context,
+ struct gssd_k5_kt_princ *ple)
{
if (--ple->refcount)
return;
- printerr(3, "freeing cached principal (ccname=%s, realm=%s)\n", ple->ccname, ple->realm);
+ printerr(3, "freeing cached principal (ccname=%s, realm=%s)\n",
+ ple->ccname, ple->realm);
krb5_free_principal(context, ple->princ);
free(ple->ccname);
free(ple->realm);
free(ple);
}
+static void release_ple(krb5_context context, struct gssd_k5_kt_princ *ple)
+{
+ pthread_mutex_lock(&ple_lock);
+ release_ple_locked(context, ple);
+ pthread_mutex_unlock(&ple_lock);
+}
+
+
/*
* Called from the scandir function to weed out potential krb5
* credentials cache files
@@ -1420,7 +1430,7 @@ gssd_destroy_krb5_principals(int destroy_machine_creds)
}
}
- release_ple(context, ple);
+ release_ple_locked(context, ple);
}
pthread_mutex_unlock(&ple_lock);
krb5_free_context(context);
--
2.31.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH nfs-utils] gssd: use mutex to protect decrement of refcount
2021-05-21 4:54 [PATCH nfs-utils] gssd: use mutex to protect decrement of refcount NeilBrown
@ 2021-05-23 18:28 ` Steve Dickson
0 siblings, 0 replies; 2+ messages in thread
From: Steve Dickson @ 2021-05-23 18:28 UTC (permalink / raw
To: NeilBrown; +Cc: Linux NFS Mailing list
On 5/21/21 12:54 AM, NeilBrown wrote:
>
> The decrement of the "ple" refcount is not protected so it can race with
> increments or decrements from other threads. An increment could be lost
> and then the ple would be freed early, leading to memory corruption.
>
> So use the mutex to protect decrements (increments are already
> protected).
>
> As gssd_destroy_krb5_principals() calls release_ple() while holding the
> mutex, we need a "release_pte_locked()" which doesn't take the mutex.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
Committed... (tag: nfs-utils-2-5-4-rc4)
steved.
> ---
> utils/gssd/krb5_util.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index 28b60ba307d0..51e0c6a2484b 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -169,18 +169,28 @@ static int gssd_get_single_krb5_cred(krb5_context context,
> static int query_krb5_ccache(const char* cred_cache, char **ret_princname,
> char **ret_realm);
>
> -static void release_ple(krb5_context context, struct gssd_k5_kt_princ *ple)
> +static void release_ple_locked(krb5_context context,
> + struct gssd_k5_kt_princ *ple)
> {
> if (--ple->refcount)
> return;
>
> - printerr(3, "freeing cached principal (ccname=%s, realm=%s)\n", ple->ccname, ple->realm);
> + printerr(3, "freeing cached principal (ccname=%s, realm=%s)\n",
> + ple->ccname, ple->realm);
> krb5_free_principal(context, ple->princ);
> free(ple->ccname);
> free(ple->realm);
> free(ple);
> }
>
> +static void release_ple(krb5_context context, struct gssd_k5_kt_princ *ple)
> +{
> + pthread_mutex_lock(&ple_lock);
> + release_ple_locked(context, ple);
> + pthread_mutex_unlock(&ple_lock);
> +}
> +
> +
> /*
> * Called from the scandir function to weed out potential krb5
> * credentials cache files
> @@ -1420,7 +1430,7 @@ gssd_destroy_krb5_principals(int destroy_machine_creds)
> }
> }
>
> - release_ple(context, ple);
> + release_ple_locked(context, ple);
> }
> pthread_mutex_unlock(&ple_lock);
> krb5_free_context(context);
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-05-23 18:26 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-21 4:54 [PATCH nfs-utils] gssd: use mutex to protect decrement of refcount NeilBrown
2021-05-23 18:28 ` Steve Dickson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.