| Message ID | 20200701182803.14947-5-nazard@nazar.ca (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Misc fixes & cleanups for nfs-utils | expand |
Hello, On 7/1/20 2:27 PM, Doug Nazar wrote: > Signed-off-by: Doug Nazar <nazard@nazar.ca> > --- > utils/gssd/krb5_util.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c > index c49c6672..b1e48241 100644 > --- a/utils/gssd/krb5_util.c > +++ b/utils/gssd/krb5_util.c > @@ -484,7 +484,7 @@ gssd_get_single_krb5_cred(krb5_context context, > if (ccache) > krb5_cc_close(context, ccache); > krb5_free_cred_contents(context, &my_creds); > - krb5_free_string(context, k5err); > + free(k5err); > return (code); > } > > @@ -723,7 +723,7 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt, > "we failed to unparse principal name: %s\n", > k5err); > k5_free_kt_entry(context, kte); > - krb5_free_string(context, k5err); > + free(k5err); > k5err = NULL; > continue; > } > @@ -770,7 +770,7 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt, > if (retval < 0) > retval = 0; > out: > - krb5_free_string(context, k5err); > + free(k5err); > return retval; > } > > @@ -927,7 +927,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, > k5err = gssd_k5_err_msg(context, code); > printerr(1, "%s while building principal for '%s'\n", > k5err, spn); > - krb5_free_string(context, k5err); > + free(k5err); > k5err = NULL; > continue; > } > @@ -937,7 +937,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, > k5err = gssd_k5_err_msg(context, code); > printerr(3, "%s while getting keytab entry for '%s'\n", > k5err, spn); > - krb5_free_string(context, k5err); > + free(k5err); > k5err = NULL; > /* > * We tried the active directory machine account > @@ -986,7 +986,7 @@ out: > k5_free_default_realm(context, default_realm); > if (realmnames) > krb5_free_host_realm(context, realmnames); > - krb5_free_string(context, k5err); > + free(k5err); > return retval; > } > > @@ -1355,7 +1355,7 @@ out_free_kt: > out_free_context: > krb5_free_context(context); > out: > - krb5_free_string(context, k5err); > + free(k5err); > return retval; > } > > I'm curious about these changes... since all krb5_free_string() does is call free()... where is the "strdup'd msg" coming from? steved.
On 2020-07-08 10:50, Steve Dickson wrote: > I'm curious about these changes... since all krb5_free_string() > does is call free()... where is the "strdup'd msg" coming from? gssd_k5_err_msg() always returns a local strdup() of the error message. We shouldn't be using a Kerberos library method to free them. There's no guarantee that the library won't change, or even that the library was compiled with the same malloc library. Doug
On 7/12/20 4:27 PM, Doug Nazar wrote: > On 2020-07-08 10:50, Steve Dickson wrote: >> I'm curious about these changes... since all krb5_free_string() >> does is call free()... where is the "strdup'd msg" coming from? > > gssd_k5_err_msg() always returns a local strdup() of the error message. True... > We shouldn't be using a Kerberos library method to free them. There's no guarantee that the library won't change, I guess I'm not too worry about this... I would the change was for the better. and lets face the krb5 has not changed in a 100 years ;-) > or even that the library was compiled with the same malloc library. There are different malloc libraries other than glibc? steved.
On 2020-07-13 14:47, Steve Dickson wrote: > or even that the library was compiled with the same malloc library. > There are different malloc libraries other than glibc? Not including the various libc implementations (dietlibc, klibc, uclibc) there are several specialized malloc libraries (dmalloc & jemalloc are the most common) that you can compile or link against. Usually since they replace malloc and friends everything works, but you can get into issues depending on order of linking and how you compile. I recently put gssd though a few rounds of mallocfail, which allows you to progressive test every allocation point (during normal running) to ensure it's handled (instead of crashing). I've an updated patchset that fixes the event2 conversion to check for successful allocation and a couple other spots that crashed if the allocation failed. I'm taking it a bit slower this time... kinda rushed the last patchset out the door since I was needed on another project. ;-) Doug
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c index c49c6672..b1e48241 100644 --- a/utils/gssd/krb5_util.c +++ b/utils/gssd/krb5_util.c @@ -484,7 +484,7 @@ gssd_get_single_krb5_cred(krb5_context context, if (ccache) krb5_cc_close(context, ccache); krb5_free_cred_contents(context, &my_creds); - krb5_free_string(context, k5err); + free(k5err); return (code); } @@ -723,7 +723,7 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt, "we failed to unparse principal name: %s\n", k5err); k5_free_kt_entry(context, kte); - krb5_free_string(context, k5err); + free(k5err); k5err = NULL; continue; } @@ -770,7 +770,7 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt, if (retval < 0) retval = 0; out: - krb5_free_string(context, k5err); + free(k5err); return retval; } @@ -927,7 +927,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, k5err = gssd_k5_err_msg(context, code); printerr(1, "%s while building principal for '%s'\n", k5err, spn); - krb5_free_string(context, k5err); + free(k5err); k5err = NULL; continue; } @@ -937,7 +937,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, k5err = gssd_k5_err_msg(context, code); printerr(3, "%s while getting keytab entry for '%s'\n", k5err, spn); - krb5_free_string(context, k5err); + free(k5err); k5err = NULL; /* * We tried the active directory machine account @@ -986,7 +986,7 @@ out: k5_free_default_realm(context, default_realm); if (realmnames) krb5_free_host_realm(context, realmnames); - krb5_free_string(context, k5err); + free(k5err); return retval; } @@ -1355,7 +1355,7 @@ out_free_kt: out_free_context: krb5_free_context(context); out: - krb5_free_string(context, k5err); + free(k5err); return retval; }
Signed-off-by: Doug Nazar <nazard@nazar.ca> --- utils/gssd/krb5_util.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)