* [PATCH 0/4] nfs-utils: A series of memory fixes @ 2021-08-06 12:27 Alice Mitchell 2021-08-06 12:31 ` [PATCH 1/4] nfs-utils: Fix potential memory leaks in idmap Alice Mitchell ` (4 more replies) 0 siblings, 5 replies; 11+ messages in thread From: Alice Mitchell @ 2021-08-06 12:27 UTC (permalink / raw) To: linux-nfs; +Cc: Steve Dickson This series of patches fix a number of potential memory leaks and memory errors within nfs-utils that mostly happen under various error conditions. Signed-off-by: Alice Mitchell <ajmitchell@redhat.com> Alice Mitchell (4): nfs-utils: Fix potential memory leaks in idmap nfs-utils: Fix mem leaks in gssd nfs-utils: Fix mem leaks in krb5_util nfs-utils: Fix mem leak in mountd support/nfsidmap/nss.c | 4 ++-- support/nfsidmap/regex.c | 1 + utils/gssd/gssd.c | 10 +++++----- utils/gssd/krb5_util.c | 18 ++++++++++++++++-- utils/mountd/rmtab.c | 3 +++ 5 files changed, 27 insertions(+), 9 deletions(-) -- 2.27.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] nfs-utils: Fix potential memory leaks in idmap 2021-08-06 12:27 [PATCH 0/4] nfs-utils: A series of memory fixes Alice Mitchell @ 2021-08-06 12:31 ` Alice Mitchell 2021-08-06 19:14 ` Olga Kornievskaia 2021-08-06 12:32 ` [PATCH 2/4] nfs-utils: Fix mem leaks in gssd Alice Mitchell ` (3 subsequent siblings) 4 siblings, 1 reply; 11+ messages in thread From: Alice Mitchell @ 2021-08-06 12:31 UTC (permalink / raw) To: linux-nfs; +Cc: Steve Dickson Signed-off-by: Alice Mitchell <ajmitchell@redhat.com> --- support/nfsidmap/nss.c | 4 ++-- support/nfsidmap/regex.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/support/nfsidmap/nss.c b/support/nfsidmap/nss.c index 669760b..f00bd9b 100644 --- a/support/nfsidmap/nss.c +++ b/support/nfsidmap/nss.c @@ -365,9 +365,9 @@ static int _nss_name_to_gid(char *name, gid_t *gid, int dostrip) out_buf: free(buf); out_name: - if (dostrip) + if (localname) free(localname); - if (get_reformat_group()) + if (ref_name) free(ref_name); out: return err; diff --git a/support/nfsidmap/regex.c b/support/nfsidmap/regex.c index fdbb2e2..958b4ac 100644 --- a/support/nfsidmap/regex.c +++ b/support/nfsidmap/regex.c @@ -157,6 +157,7 @@ again: IDMAP_LOG(4, ("regexp_getpwnam: name '%s' mapped to '%s'", name, localname)); + free(localname); *err_p = 0; return pw; -- 2.27.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] nfs-utils: Fix potential memory leaks in idmap 2021-08-06 12:31 ` [PATCH 1/4] nfs-utils: Fix potential memory leaks in idmap Alice Mitchell @ 2021-08-06 19:14 ` Olga Kornievskaia 0 siblings, 0 replies; 11+ messages in thread From: Olga Kornievskaia @ 2021-08-06 19:14 UTC (permalink / raw) To: Alice Mitchell; +Cc: linux-nfs, Steve Dickson On Fri, Aug 6, 2021 at 12:21 PM Alice Mitchell <ajmitchell@redhat.com> wrote: > > > Signed-off-by: Alice Mitchell <ajmitchell@redhat.com> > --- > support/nfsidmap/nss.c | 4 ++-- > support/nfsidmap/regex.c | 1 + > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/support/nfsidmap/nss.c b/support/nfsidmap/nss.c > index 669760b..f00bd9b 100644 > --- a/support/nfsidmap/nss.c > +++ b/support/nfsidmap/nss.c > @@ -365,9 +365,9 @@ static int _nss_name_to_gid(char *name, gid_t *gid, > int dostrip) > out_buf: > free(buf); > out_name: > - if (dostrip) > + if (localname) > free(localname); > - if (get_reformat_group()) > + if (ref_name) > free(ref_name); Do we even need to check for null before freeing these days? man page says if null is passed then it's a no-op. If we are not allowed to free a null then there is another patch in the series in the mountd code that does intentionally do a free of null pointers. > out: > return err; > diff --git a/support/nfsidmap/regex.c b/support/nfsidmap/regex.c > index fdbb2e2..958b4ac 100644 > --- a/support/nfsidmap/regex.c > +++ b/support/nfsidmap/regex.c > @@ -157,6 +157,7 @@ again: > IDMAP_LOG(4, ("regexp_getpwnam: name '%s' mapped to '%s'", > name, localname)); > > + free(localname); > *err_p = 0; > return pw; > > -- > 2.27.0 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] nfs-utils: Fix mem leaks in gssd 2021-08-06 12:27 [PATCH 0/4] nfs-utils: A series of memory fixes Alice Mitchell 2021-08-06 12:31 ` [PATCH 1/4] nfs-utils: Fix potential memory leaks in idmap Alice Mitchell @ 2021-08-06 12:32 ` Alice Mitchell 2021-08-06 19:12 ` Olga Kornievskaia 2021-08-06 12:33 ` [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util Alice Mitchell ` (2 subsequent siblings) 4 siblings, 1 reply; 11+ messages in thread From: Alice Mitchell @ 2021-08-06 12:32 UTC (permalink / raw) To: linux-nfs; +Cc: Steve Dickson Also fix the modification of a returned config value which should be treated as const. Signed-off-by: Alice Mitchell <ajmitchell@redhat.com> --- utils/gssd/gssd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c index 4113cba..0815665 100644 --- a/utils/gssd/gssd.c +++ b/utils/gssd/gssd.c @@ -1016,7 +1016,7 @@ read_gss_conf(void) keytabfile = s; s = conf_get_str("gssd", "cred-cache-directory"); if (s) - ccachedir = s; + ccachedir = strdup(s); s = conf_get_str("gssd", "preferred-realm"); if (s) preferred_realm = s; @@ -1070,7 +1070,7 @@ main(int argc, char *argv[]) keytabfile = optarg; break; case 'd': - ccachedir = optarg; + ccachedir = strdup(optarg); break; case 't': context_timeout = atoi(optarg); @@ -1133,7 +1133,6 @@ main(int argc, char *argv[]) } if (ccachedir) { - char *ccachedir_copy; char *ptr; for (ptr = ccachedir, i = 2; *ptr; ptr++) @@ -1141,8 +1140,7 @@ main(int argc, char *argv[]) i++; ccachesearch = malloc(i * sizeof(char *)); - ccachedir_copy = strdup(ccachedir); - if (!ccachedir_copy || !ccachesearch) { + if (!ccachesearch) { printerr(0, "malloc failure\n"); exit(EXIT_FAILURE); } @@ -1274,6 +1272,8 @@ main(int argc, char *argv[]) free(preferred_realm); free(ccachesearch); + if (ccachedir) + free(ccachedir); return rc < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } -- 2.27.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] nfs-utils: Fix mem leaks in gssd 2021-08-06 12:32 ` [PATCH 2/4] nfs-utils: Fix mem leaks in gssd Alice Mitchell @ 2021-08-06 19:12 ` Olga Kornievskaia 2021-08-12 13:19 ` Alice Mitchell 0 siblings, 1 reply; 11+ messages in thread From: Olga Kornievskaia @ 2021-08-06 19:12 UTC (permalink / raw) To: Alice Mitchell; +Cc: linux-nfs, Steve Dickson On Fri, Aug 6, 2021 at 12:23 PM Alice Mitchell <ajmitchell@redhat.com> wrote: > > Also fix the modification of a returned config value which > should be treated as const. > > Signed-off-by: Alice Mitchell <ajmitchell@redhat.com> > --- > utils/gssd/gssd.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c > index 4113cba..0815665 100644 > --- a/utils/gssd/gssd.c > +++ b/utils/gssd/gssd.c > @@ -1016,7 +1016,7 @@ read_gss_conf(void) > keytabfile = s; > s = conf_get_str("gssd", "cred-cache-directory"); > if (s) > - ccachedir = s; > + ccachedir = strdup(s); > s = conf_get_str("gssd", "preferred-realm"); > if (s) > preferred_realm = s; > @@ -1070,7 +1070,7 @@ main(int argc, char *argv[]) > keytabfile = optarg; > break; > case 'd': > - ccachedir = optarg; > + ccachedir = strdup(optarg); Is it possible that there will be a value in both config file and command line args. If we are strdup-ing in both we'll be over-writting and leaking memory? Why do we need to malloc it at all? Is it ever malloc-ed now (and considered a leak)? I think in both cases it uses static memory and doesnt require freeing. > break; > case 't': > context_timeout = atoi(optarg); > @@ -1133,7 +1133,6 @@ main(int argc, char *argv[]) > } > > if (ccachedir) { > - char *ccachedir_copy; > char *ptr; > > for (ptr = ccachedir, i = 2; *ptr; ptr++) > @@ -1141,8 +1140,7 @@ main(int argc, char *argv[]) > i++; > > ccachesearch = malloc(i * sizeof(char *)); > - ccachedir_copy = strdup(ccachedir); > - if (!ccachedir_copy || !ccachesearch) { > + if (!ccachesearch) { ccachedir_copy is the only leak here. > printerr(0, "malloc failure\n"); > exit(EXIT_FAILURE); > } > @@ -1274,6 +1272,8 @@ main(int argc, char *argv[]) > > free(preferred_realm); > free(ccachesearch); > + if (ccachedir) > + free(ccachedir); > > return rc < 0 ? EXIT_FAILURE : EXIT_SUCCESS; > } > -- > 2.27.0 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] nfs-utils: Fix mem leaks in gssd 2021-08-06 19:12 ` Olga Kornievskaia @ 2021-08-12 13:19 ` Alice Mitchell 0 siblings, 0 replies; 11+ messages in thread From: Alice Mitchell @ 2021-08-12 13:19 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: linux-nfs, Steve Dickson On Fri, 2021-08-06 at 15:12 -0400, Olga Kornievskaia wrote: > On Fri, Aug 6, 2021 at 12:23 PM Alice Mitchell <ajmitchell@redhat.com > > wrote: > > Also fix the modification of a returned config value which > > should be treated as const. > > > > Signed-off-by: Alice Mitchell <ajmitchell@redhat.com> > > --- > > utils/gssd/gssd.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c > > index 4113cba..0815665 100644 > > --- a/utils/gssd/gssd.c > > +++ b/utils/gssd/gssd.c > > @@ -1016,7 +1016,7 @@ read_gss_conf(void) > > keytabfile = s; > > s = conf_get_str("gssd", "cred-cache-directory"); > > if (s) > > - ccachedir = s; > > + ccachedir = strdup(s); > > s = conf_get_str("gssd", "preferred-realm"); > > if (s) > > preferred_realm = s; > > @@ -1070,7 +1070,7 @@ main(int argc, char *argv[]) > > keytabfile = optarg; > > break; > > case 'd': > > - ccachedir = optarg; > > + ccachedir = strdup(optarg); > > Is it possible that there will be a value in both config file and > command line args. If we are strdup-ing in both we'll be over- > writting > and leaking memory? > > Why do we need to malloc it at all? Is it ever malloc-ed now (and > considered a leak)? I think in both cases it uses static memory and > doesnt require freeing. in both cases the string pointed to gets modified by a later strtok() and so will be modifying the original of argv or a conf parameter, which i presume is why there was ccacherdir_copy to avoid doing that, but it was never properly utilised. i guess strtok truncating those strings doesnt actually *hurt* anything right now, its just a case of unexpected side-effects should anyone later try to reuse them. > > > break; > > case 't': > > context_timeout = atoi(optarg); > > @@ -1133,7 +1133,6 @@ main(int argc, char *argv[]) > > } > > > > if (ccachedir) { > > - char *ccachedir_copy; > > char *ptr; > > > > for (ptr = ccachedir, i = 2; *ptr; ptr++) > > @@ -1141,8 +1140,7 @@ main(int argc, char *argv[]) > > i++; > > > > ccachesearch = malloc(i * sizeof(char *)); > > - ccachedir_copy = strdup(ccachedir); > > - if (!ccachedir_copy || !ccachesearch) { > > + if (!ccachesearch) { > > ccachedir_copy is the only leak here. > > > printerr(0, "malloc failure\n"); > > exit(EXIT_FAILURE); > > } > > @@ -1274,6 +1272,8 @@ main(int argc, char *argv[]) > > > > free(preferred_realm); > > free(ccachesearch); > > + if (ccachedir) > > + free(ccachedir); > > > > return rc < 0 ? EXIT_FAILURE : EXIT_SUCCESS; > > } > > -- > > 2.27.0 > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util 2021-08-06 12:27 [PATCH 0/4] nfs-utils: A series of memory fixes Alice Mitchell 2021-08-06 12:31 ` [PATCH 1/4] nfs-utils: Fix potential memory leaks in idmap Alice Mitchell 2021-08-06 12:32 ` [PATCH 2/4] nfs-utils: Fix mem leaks in gssd Alice Mitchell @ 2021-08-06 12:33 ` Alice Mitchell 2021-08-06 18:49 ` Olga Kornievskaia 2021-08-06 12:33 ` [PATCH 4/4] nfs-utils: Fix mem leak in mountd Alice Mitchell 2021-08-06 13:47 ` [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util Alice Mitchell 4 siblings, 1 reply; 11+ messages in thread From: Alice Mitchell @ 2021-08-06 12:33 UTC (permalink / raw) To: linux-nfs; +Cc: Steve Dickson ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util 2021-08-06 12:33 ` [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util Alice Mitchell @ 2021-08-06 18:49 ` Olga Kornievskaia 0 siblings, 0 replies; 11+ messages in thread From: Olga Kornievskaia @ 2021-08-06 18:49 UTC (permalink / raw) To: Alice Mitchell; +Cc: linux-nfs, Steve Dickson Empty patch? On Fri, Aug 6, 2021 at 12:25 PM Alice Mitchell <ajmitchell@redhat.com> wrote: > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] nfs-utils: Fix mem leak in mountd 2021-08-06 12:27 [PATCH 0/4] nfs-utils: A series of memory fixes Alice Mitchell ` (2 preceding siblings ...) 2021-08-06 12:33 ` [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util Alice Mitchell @ 2021-08-06 12:33 ` Alice Mitchell 2021-08-06 13:47 ` [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util Alice Mitchell 4 siblings, 0 replies; 11+ messages in thread From: Alice Mitchell @ 2021-08-06 12:33 UTC (permalink / raw) To: linux-nfs; +Cc: Steve Dickson Signed-off-by: Alice Mitchell <ajmitchell@redhat.com> --- utils/mountd/rmtab.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/utils/mountd/rmtab.c b/utils/mountd/rmtab.c index 2da9761..752fdb6 100644 --- a/utils/mountd/rmtab.c +++ b/utils/mountd/rmtab.c @@ -233,6 +233,9 @@ mountlist_list(void) m->ml_directory = strdup(rep->r_path); if (m->ml_hostname == NULL || m->ml_directory == NULL) { + free(m->ml_hostname); + free(m->ml_directory); + free(m); mountlist_freeall(mlist); mlist = NULL; xlog(L_ERROR, "%s: memory allocation failed", -- 2.27.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util 2021-08-06 12:27 [PATCH 0/4] nfs-utils: A series of memory fixes Alice Mitchell ` (3 preceding siblings ...) 2021-08-06 12:33 ` [PATCH 4/4] nfs-utils: Fix mem leak in mountd Alice Mitchell @ 2021-08-06 13:47 ` Alice Mitchell 2021-08-06 18:53 ` Olga Kornievskaia 4 siblings, 1 reply; 11+ messages in thread From: Alice Mitchell @ 2021-08-06 13:47 UTC (permalink / raw) To: linux-nfs; +Cc: Steve Dickson ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util 2021-08-06 13:47 ` [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util Alice Mitchell @ 2021-08-06 18:53 ` Olga Kornievskaia 0 siblings, 0 replies; 11+ messages in thread From: Olga Kornievskaia @ 2021-08-06 18:53 UTC (permalink / raw) To: Alice Mitchell; +Cc: linux-nfs, Steve Dickson Still empty for me... :-/ On Fri, Aug 6, 2021 at 2:50 PM Alice Mitchell <ajmitchell@redhat.com> wrote: > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-08-12 13:22 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-06 12:27 [PATCH 0/4] nfs-utils: A series of memory fixes Alice Mitchell 2021-08-06 12:31 ` [PATCH 1/4] nfs-utils: Fix potential memory leaks in idmap Alice Mitchell 2021-08-06 19:14 ` Olga Kornievskaia 2021-08-06 12:32 ` [PATCH 2/4] nfs-utils: Fix mem leaks in gssd Alice Mitchell 2021-08-06 19:12 ` Olga Kornievskaia 2021-08-12 13:19 ` Alice Mitchell 2021-08-06 12:33 ` [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util Alice Mitchell 2021-08-06 18:49 ` Olga Kornievskaia 2021-08-06 12:33 ` [PATCH 4/4] nfs-utils: Fix mem leak in mountd Alice Mitchell 2021-08-06 13:47 ` [PATCH 3/4] nfs-utils: Fix mem leaks in krb5_util Alice Mitchell 2021-08-06 18:53 ` Olga Kornievskaia
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.