2019-08-26 15:31:06

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 1/2] nfs-utils: Removed a number of Coverity Scan RESOURCE_LEAK errors

Signed-off-by: Steve Dickson <[email protected]>
---
support/nfsidmap/libnfsidmap.c | 3 +++
utils/gssd/krb5_util.c | 4 ++++
2 files changed, 7 insertions(+)

diff --git a/support/nfsidmap/libnfsidmap.c b/support/nfsidmap/libnfsidmap.c
index 7b8a871..9299e65 100644
--- a/support/nfsidmap/libnfsidmap.c
+++ b/support/nfsidmap/libnfsidmap.c
@@ -486,6 +486,9 @@ out:
if (gss_methods)
conf_free_list(gss_methods);

+ if (nfs4_methods)
+ conf_free_list(nfs4_methods);
+
return ret ? -ENOENT: 0;
}

diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index 454a6eb..f68be85 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -912,6 +912,8 @@ 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);
+ free(k5err);
+ k5err = NULL;
/*
* We tried the active directory machine account
* with the hostname part as-is and failed...
@@ -1231,6 +1233,8 @@ gssd_destroy_krb5_machine_creds(void)
k5err = gssd_k5_err_msg(context, code);
printerr(0, "WARNING: %s while destroying credential "
"cache '%s'\n", k5err, ple->ccname);
+ free(k5err);
+ k5err = NULL;
}
}
krb5_free_context(context);
--
2.21.0


2019-08-26 15:31:06

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 2/2] nfs-utils: Removed a number of Coverity Scan USE_AFTER_FREE errors

Signed-off-by: Steve Dickson <[email protected]>
---
utils/gssd/krb5_util.c | 4 +++-
utils/idmapd/idmapd.c | 6 ++++--
utils/statd/monitor.c | 5 ++++-
utils/statd/notlist.c | 9 +++++++--
4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index f68be85..0474783 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -1016,6 +1016,8 @@ query_krb5_ccache(const char* cred_cache, char **ret_princname,
char *str = NULL;
char *princstring;

+ *ret_princname = *ret_realm = NULL;
+
ret = krb5_init_context(&context);
if (ret)
return 0;
@@ -1050,7 +1052,7 @@ err_princ:
krb5_cc_close(context, ccache);
err_cache:
krb5_free_context(context);
- return found;
+ return (*ret_princname && *ret_realm);
}

/*==========================*/
diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index 267acea..c187e7d 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -520,14 +520,16 @@ static void
clntscancb(int UNUSED(fd), short UNUSED(which), void *data)
{
struct idmap_clientq *icq = data;
- struct idmap_client *ic;
+ struct idmap_client *ic, *ic_next;

- TAILQ_FOREACH(ic, icq, ic_next)
+ for (ic = TAILQ_FIRST(icq); ic != NULL; ic = ic_next) {
+ ic_next = TAILQ_NEXT(ic, ic_next);
if (ic->ic_fd == -1 && nfsopen(ic) == -1) {
close(ic->ic_dirfd);
TAILQ_REMOVE(icq, ic, ic_next);
free(ic);
}
+ }
}

static void
diff --git a/utils/statd/monitor.c b/utils/statd/monitor.c
index 9400048..20c8ebd 100644
--- a/utils/statd/monitor.c
+++ b/utils/statd/monitor.c
@@ -66,7 +66,7 @@ sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp)
*my_name = argp->mon_id.my_id.my_name;
struct my_id *id = &argp->mon_id.my_id;
char *cp;
- notify_list *clnt;
+ notify_list *clnt = NULL;
struct sockaddr_in my_addr = {
.sin_family = AF_INET,
.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
@@ -177,6 +177,7 @@ sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp)
* We're committed...ignoring errors. Let's hope that a malloc()
* doesn't fail. (I should probably fix this assumption.)
*/
+ clnt = NULL;
if (!existing && !(clnt = nlist_new(my_name, mon_name, 0))) {
free(dnsname);
xlog_warn("out of memory");
@@ -223,6 +224,7 @@ sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp)

failure:
xlog_warn("STAT_FAIL to %s for SM_MON of %s", my_name, mon_name);
+ free(clnt);
return (&result);
}

@@ -242,6 +244,7 @@ load_one_host(const char *hostname,
clnt->dns_name = strdup(hostname);
if (clnt->dns_name == NULL) {
nlist_free(NULL, clnt);
+ free(clnt);
return 0;
}

diff --git a/utils/statd/notlist.c b/utils/statd/notlist.c
index 0341c15..45879a4 100644
--- a/utils/statd/notlist.c
+++ b/utils/statd/notlist.c
@@ -210,7 +210,6 @@ nlist_free(notify_list **head, notify_list *entry)
if (NL_MON_NAME(entry))
free(NL_MON_NAME(entry));
free(entry->dns_name);
- free(entry);
}

/*
@@ -219,8 +218,14 @@ nlist_free(notify_list **head, notify_list *entry)
void
nlist_kill(notify_list **head)
{
- while (*head)
+ notify_list *next;
+
+ while (*head) {
+ next = (*head)->next;
nlist_free(head, *head);
+ free(*head);
+ *head = next;
+ }
}

/*
--
2.21.0

2019-08-26 18:49:47

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs-utils: Removed a number of Coverity Scan RESOURCE_LEAK errors



On 8/26/19 10:34 AM, Steve Dickson wrote:
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> support/nfsidmap/libnfsidmap.c | 3 +++
> utils/gssd/krb5_util.c | 4 ++++
> 2 files changed, 7 insertions(+)
Committed...

steved.

>
> diff --git a/support/nfsidmap/libnfsidmap.c b/support/nfsidmap/libnfsidmap.c
> index 7b8a871..9299e65 100644
> --- a/support/nfsidmap/libnfsidmap.c
> +++ b/support/nfsidmap/libnfsidmap.c
> @@ -486,6 +486,9 @@ out:
> if (gss_methods)
> conf_free_list(gss_methods);
>
> + if (nfs4_methods)
> + conf_free_list(nfs4_methods);
> +
> return ret ? -ENOENT: 0;
> }
>
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index 454a6eb..f68be85 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -912,6 +912,8 @@ 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);
> + free(k5err);
> + k5err = NULL;
> /*
> * We tried the active directory machine account
> * with the hostname part as-is and failed...
> @@ -1231,6 +1233,8 @@ gssd_destroy_krb5_machine_creds(void)
> k5err = gssd_k5_err_msg(context, code);
> printerr(0, "WARNING: %s while destroying credential "
> "cache '%s'\n", k5err, ple->ccname);
> + free(k5err);
> + k5err = NULL;
> }
> }
> krb5_free_context(context);
>

2019-08-26 18:50:24

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/2] nfs-utils: Removed a number of Coverity Scan USE_AFTER_FREE errors



On 8/26/19 10:34 AM, Steve Dickson wrote:
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> utils/gssd/krb5_util.c | 4 +++-
> utils/idmapd/idmapd.c | 6 ++++--
> utils/statd/monitor.c | 5 ++++-
> utils/statd/notlist.c | 9 +++++++--
> 4 files changed, 18 insertions(+), 6 deletions(-)
Committed...

steved.
>
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index f68be85..0474783 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -1016,6 +1016,8 @@ query_krb5_ccache(const char* cred_cache, char **ret_princname,
> char *str = NULL;
> char *princstring;
>
> + *ret_princname = *ret_realm = NULL;
> +
> ret = krb5_init_context(&context);
> if (ret)
> return 0;
> @@ -1050,7 +1052,7 @@ err_princ:
> krb5_cc_close(context, ccache);
> err_cache:
> krb5_free_context(context);
> - return found;
> + return (*ret_princname && *ret_realm);
> }
>
> /*==========================*/
> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
> index 267acea..c187e7d 100644
> --- a/utils/idmapd/idmapd.c
> +++ b/utils/idmapd/idmapd.c
> @@ -520,14 +520,16 @@ static void
> clntscancb(int UNUSED(fd), short UNUSED(which), void *data)
> {
> struct idmap_clientq *icq = data;
> - struct idmap_client *ic;
> + struct idmap_client *ic, *ic_next;
>
> - TAILQ_FOREACH(ic, icq, ic_next)
> + for (ic = TAILQ_FIRST(icq); ic != NULL; ic = ic_next) {
> + ic_next = TAILQ_NEXT(ic, ic_next);
> if (ic->ic_fd == -1 && nfsopen(ic) == -1) {
> close(ic->ic_dirfd);
> TAILQ_REMOVE(icq, ic, ic_next);
> free(ic);
> }
> + }
> }
>
> static void
> diff --git a/utils/statd/monitor.c b/utils/statd/monitor.c
> index 9400048..20c8ebd 100644
> --- a/utils/statd/monitor.c
> +++ b/utils/statd/monitor.c
> @@ -66,7 +66,7 @@ sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp)
> *my_name = argp->mon_id.my_id.my_name;
> struct my_id *id = &argp->mon_id.my_id;
> char *cp;
> - notify_list *clnt;
> + notify_list *clnt = NULL;
> struct sockaddr_in my_addr = {
> .sin_family = AF_INET,
> .sin_addr.s_addr = htonl(INADDR_LOOPBACK),
> @@ -177,6 +177,7 @@ sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp)
> * We're committed...ignoring errors. Let's hope that a malloc()
> * doesn't fail. (I should probably fix this assumption.)
> */
> + clnt = NULL;
> if (!existing && !(clnt = nlist_new(my_name, mon_name, 0))) {
> free(dnsname);
> xlog_warn("out of memory");
> @@ -223,6 +224,7 @@ sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp)
>
> failure:
> xlog_warn("STAT_FAIL to %s for SM_MON of %s", my_name, mon_name);
> + free(clnt);
> return (&result);
> }
>
> @@ -242,6 +244,7 @@ load_one_host(const char *hostname,
> clnt->dns_name = strdup(hostname);
> if (clnt->dns_name == NULL) {
> nlist_free(NULL, clnt);
> + free(clnt);
> return 0;
> }
>
> diff --git a/utils/statd/notlist.c b/utils/statd/notlist.c
> index 0341c15..45879a4 100644
> --- a/utils/statd/notlist.c
> +++ b/utils/statd/notlist.c
> @@ -210,7 +210,6 @@ nlist_free(notify_list **head, notify_list *entry)
> if (NL_MON_NAME(entry))
> free(NL_MON_NAME(entry));
> free(entry->dns_name);
> - free(entry);
> }
>
> /*
> @@ -219,8 +218,14 @@ nlist_free(notify_list **head, notify_list *entry)
> void
> nlist_kill(notify_list **head)
> {
> - while (*head)
> + notify_list *next;
> +
> + while (*head) {
> + next = (*head)->next;
> nlist_free(head, *head);
> + free(*head);
> + *head = next;
> + }
> }
>
> /*
>