2012-01-02 15:56:02

by Jim Meyering

Subject: [PATCH] gssd: avoid double-free upon write failure

* utils/gssd/context_lucid.c (prepare_krb5_rfc1964_buffer): Free just-
calloc'd enc_key.data from one place, unconditionally, after calling
write_lucid_keyblock, rather than from three places. Before, upon
failed write, we would free it, then goto out_err, where we would
free it again, if it happened to be non-NULL.
Coverity spotted the possible double free.
---
utils/gssd/context_lucid.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/utils/gssd/context_lucid.c b/utils/gssd/context_lucid.c
index 3e695ab..64146d7 100644
--- a/utils/gssd/context_lucid.c
+++ b/utils/gssd/context_lucid.c
@@ -80,6 +80,7 @@ prepare_krb5_rfc1964_buffer(gss_krb5_lucid_context_v1_t *lctx,
uint32_t i;
char *skd, *dkd;
gss_buffer_desc fakeoid;
+ int err;

/*
* The new Kerberos interface to get the gss context
@@ -138,11 +139,10 @@ prepare_krb5_rfc1964_buffer(gss_krb5_lucid_context_v1_t *lctx,
dkd = (char *) enc_key.data;
for (i = 0; i < enc_key.length; i++)
dkd[i] = skd[i] ^ 0xf0;
- if (write_lucid_keyblock(&p, end, &enc_key)) {
- free(enc_key.data);
- goto out_err;
- }
+ err = write_lucid_keyblock(&p, end, &enc_key);
free(enc_key.data);
+ if (err)
+ goto out_err;

if (write_lucid_keyblock(&p, end, &lctx->rfc1964_kd.ctx_key))
goto out_err;
@@ -153,7 +153,6 @@ out_err:
printerr(0, "ERROR: failed serializing krb5 context for kernel\n");
if (buf->value) free(buf->value);
buf->length = 0;
- if (enc_key.data) free(enc_key.data);
return -1;
}

--
1.7.8.1.391.g2c2ad

2012-01-05 21:43:09

by Steve Dickson

Subject: Re: [PATCH] gssd: avoid double-free upon write failure

On 01/02/2012 10:55 AM, Jim Meyering wrote:
>
> * utils/gssd/context_lucid.c (prepare_krb5_rfc1964_buffer): Free just-
> calloc'd enc_key.data from one place, unconditionally, after calling
> write_lucid_keyblock, rather than from three places. Before, upon
> failed write, we would free it, then goto out_err, where we would
> free it again, if it happened to be non-NULL.
> Coverity spotted the possible double free.
Committed....

steved.

> ---
> utils/gssd/context_lucid.c | 9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/utils/gssd/context_lucid.c b/utils/gssd/context_lucid.c
> index 3e695ab..64146d7 100644
> --- a/utils/gssd/context_lucid.c
> +++ b/utils/gssd/context_lucid.c
> @@ -80,6 +80,7 @@ prepare_krb5_rfc1964_buffer(gss_krb5_lucid_context_v1_t *lctx,
> uint32_t i;
> char *skd, *dkd;
> gss_buffer_desc fakeoid;
> + int err;
>
> /*
> * The new Kerberos interface to get the gss context
> @@ -138,11 +139,10 @@ prepare_krb5_rfc1964_buffer(gss_krb5_lucid_context_v1_t *lctx,
> dkd = (char *) enc_key.data;
> for (i = 0; i < enc_key.length; i++)
> dkd[i] = skd[i] ^ 0xf0;
> - if (write_lucid_keyblock(&p, end, &enc_key)) {
> - free(enc_key.data);
> - goto out_err;
> - }
> + err = write_lucid_keyblock(&p, end, &enc_key);
> free(enc_key.data);
> + if (err)
> + goto out_err;
>
> if (write_lucid_keyblock(&p, end, &lctx->rfc1964_kd.ctx_key))
> goto out_err;
> @@ -153,7 +153,6 @@ out_err:
> printerr(0, "ERROR: failed serializing krb5 context for kernel\n");
> if (buf->value) free(buf->value);
> buf->length = 0;
> - if (enc_key.data) free(enc_key.data);
> return -1;
> }
>
> --
> 1.7.8.1.391.g2c2ad
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html