* [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
* [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
* [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
* [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 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
* 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
* 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 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
* 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
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.