Red Hat is now requiring covertity scans
to be run against all RHEL 8 packages.
These patches removed the majority of the
resource leaks that were flagged by the scan.
Most of the leaks were in return and error
paths as well as some obvious problems like
checking the wrong point to be NULL.
There are still a few resources leaks
and used_after_freed being flagged but
I am thinking they false-positives
because I just don't see the problem.
I've tested these patches for a couple
days and they seem stable... but whenever
free()s are added... So is risk of freeing
that is still being used. Plus they will
get a good workout at the upcoming Bakeathon.
Steve Dickson (19):
Removed resource leaks from junction/path.c
Removed resource leaks from nfs/exports.c
Removed a resource leak from nfs/mydaemon.c
Removed a resource leak from nfs/rpcmisc.c
Removed a resource leak from nfs/svc_socket.c
Removed bad frees from nfs/xcommon.c
Removed resource leaks from nfs/xlog.c
Removed resource leaks from nfsidmap/libnfsidmap.c
Removed resource leaks from nfsidmap/static.c
Removed a resource leak from nsm/file.c
Removed resource leaks from systemd/rpc-pipefs-generator.c
Removed resource leaks from blkmapd/device-discovery.c
Removed resource leaks from gssd/krb5_util.c
Removed a resource leak from mount/configfile.c
Removed a resource leak from mount/nfsmount.c
Removed a resource leak from mount/stropts.c
Removed resource leaks from mountd/cache.c
Removed a resource leak from mountd/fsloc.c
Removed a resource leak from nfsdcltrack/sqlite.c
support/junction/path.c | 6 +++++-
support/nfs/exports.c | 2 ++
support/nfs/mydaemon.c | 1 +
support/nfs/rpcmisc.c | 1 +
support/nfs/svc_socket.c | 1 +
support/nfs/xcommon.c | 14 ++++++++++----
support/nfs/xlog.c | 6 +++++-
support/nfsidmap/libnfsidmap.c | 10 ++++++++--
support/nfsidmap/static.c | 10 ++++++++++
support/nsm/file.c | 1 +
systemd/rpc-pipefs-generator.c | 10 ++++++++--
utils/blkmapd/device-discovery.c | 22 +++++++++++++++++++++-
utils/gssd/krb5_util.c | 9 ++++++++-
utils/mount/configfile.c | 2 +-
utils/mount/nfsmount.c | 1 +
utils/mount/stropts.c | 5 ++++-
utils/mountd/cache.c | 5 +++--
utils/mountd/fsloc.c | 1 +
utils/nfsdcltrack/sqlite.c | 2 ++
19 files changed, 93 insertions(+), 16 deletions(-)
--
2.20.1
mount/stropts.c:986: leaked_storage: Variable "address"
going out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
utils/mount/stropts.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index a093926..1bb7a73 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -983,8 +983,11 @@ static int nfs_try_mount(struct nfsmount_info *mi)
}
if (!nfs_append_addr_option(address->ai_addr,
- address->ai_addrlen, mi->options))
+ address->ai_addrlen, mi->options)) {
+ nfs_freeaddrinfo(address);
+ errno = ENOMEM;
return 0;
+ }
mi->address = address;
}
--
2.20.1
mountd/fsloc.c:97: overwrite_var: Overwriting "mp" in
"mp = calloc(1UL, 16UL)" leaks the storage that "mp" points to.
Signed-off-by: Steve Dickson <[email protected]>
---
utils/mountd/fsloc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/utils/mountd/fsloc.c b/utils/mountd/fsloc.c
index bc737d1..cf42944 100644
--- a/utils/mountd/fsloc.c
+++ b/utils/mountd/fsloc.c
@@ -102,6 +102,7 @@ static struct servers *parse_list(char **list)
cp = strchr(list[i], '@');
if ((!cp) || list[i][0] != '/') {
xlog(L_WARNING, "invalid entry '%s'", list[i]);
+ free(mp);
continue; /* XXX Need better error handling */
}
res->h_mp[i] = mp;
--
2.20.1
gssd/krb5_util.c:696: overwrite_var: Overwriting "k5err" in
"k5err = gssd_k5_err_msg(context, code)" leaks
the storage that "k5err" points to.
gssd/krb5_util.c:737: overwrite_var: Overwriting "k5err" in
"k5err = gssd_k5_err_msg(context, code)" leaks
the storage that "k5err" points to.
gssd/krb5_util.c:899: overwrite_var: Overwriting "k5err" in
"k5err = gssd_k5_err_msg(context, code)" leaks
the storage that "k5err" points to.
krb5_util.c:1173: leaked_storage: Variable "l" going out
of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
utils/gssd/krb5_util.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index 6daba44..454a6eb 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -699,6 +699,8 @@ 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);
+ free(k5err);
+ k5err = NULL;
continue;
}
printerr(4, "Processing keytab entry for principal '%s'\n",
@@ -900,6 +902,8 @@ 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);
+ free(k5err);
+ k5err = NULL;
continue;
}
code = krb5_kt_get_entry(context, kt, princ, 0, 0, kte);
@@ -1169,7 +1173,8 @@ gssd_get_krb5_machine_cred_list(char ***list)
*list = l;
retval = 0;
goto out;
- }
+ } else
+ free((void *)l);
out:
return retval;
}
@@ -1217,6 +1222,8 @@ gssd_destroy_krb5_machine_creds(void)
printerr(0, "WARNING: %s while resolving credential "
"cache '%s' for destruction\n", k5err,
ple->ccname);
+ free(k5err);
+ k5err = NULL;
continue;
}
--
2.20.1
mount/nfsmount.c:455: leaked_storage: Variable "mounthost" going
out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
utils/mount/nfsmount.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/utils/mount/nfsmount.c b/utils/mount/nfsmount.c
index 952a755..3d95da9 100644
--- a/utils/mount/nfsmount.c
+++ b/utils/mount/nfsmount.c
@@ -452,6 +452,7 @@ parse_options(char *old_opts, struct nfs_mount_data *data,
nfs_error(_("%s: Bad nfs mount parameter: %s\n"), progname, opt);
out_bad:
free(tmp_opts);
+ free(mounthost);
return 0;
}
--
2.20.1
mountd/cache.c:1244:3: warning: statement will never be
executed [-Wswitch-unreachable]
mountd/cache.c:1260: leaked_storage: Variable "locations"
going out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
utils/mountd/cache.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 2cb370f..bdbd190 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -1240,7 +1240,7 @@ static struct exportent *lookup_junction(char *dom, const char *pathname,
goto out;
}
status = nfs_get_basic_junction(pathname, &locations);
- switch (status) {
+ if (status) {
xlog(L_WARNING, "Dangling junction %s: %s",
pathname, strerror(status));
goto out;
@@ -1248,10 +1248,11 @@ static struct exportent *lookup_junction(char *dom, const char *pathname,
parent = lookup_parent_export(dom, pathname, ai);
if (parent == NULL)
- goto out;
+ goto free_locations;
exp = locations_to_export(locations, pathname, parent);
+free_locations:
nfs_free_locations(locations->ns_list);
free(locations);
--
2.20.1
nfsdcltrack/sqlite.c:218: leaked_storage: Variable "err" going out
of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
utils/nfsdcltrack/sqlite.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/utils/nfsdcltrack/sqlite.c b/utils/nfsdcltrack/sqlite.c
index c59f777..2801201 100644
--- a/utils/nfsdcltrack/sqlite.c
+++ b/utils/nfsdcltrack/sqlite.c
@@ -215,6 +215,8 @@ sqlite_maindb_init_v2(void)
&err);
if (ret != SQLITE_OK) {
xlog(L_ERROR, "Unable to begin transaction: %s", err);
+ if (err)
+ sqlite3_free(err);
return ret;
}
--
2.20.1
nfsidmap/libnfsidmap.c:410: leaked_storage: Variable "nfs4_methods"
going out of scope leaks the storage it points to.
ibnfsidmap.c:483: leaked_storage: Variable "gss_methods"
going out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
support/nfsidmap/libnfsidmap.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/support/nfsidmap/libnfsidmap.c b/support/nfsidmap/libnfsidmap.c
index 35ddf01..7b8a871 100644
--- a/support/nfsidmap/libnfsidmap.c
+++ b/support/nfsidmap/libnfsidmap.c
@@ -406,8 +406,10 @@ int nfs4_init_name_mapping(char *conffile)
nfs4_methods = conf_get_list("Translation", "Method");
if (nfs4_methods) {
IDMAP_LOG(1, ("libnfsidmap: processing 'Method' list"));
- if (load_plugins(nfs4_methods, &nfs4_plugins) == -1)
+ if (load_plugins(nfs4_methods, &nfs4_plugins) == -1) {
+ conf_free_list(nfs4_methods);
return -ENOENT;
+ }
} else {
struct conf_list list;
struct conf_list_node node;
@@ -475,11 +477,15 @@ out:
if (ret) {
if (nfs4_plugins)
unload_plugins(nfs4_plugins);
- if (gss_plugins)
+ if (gss_plugins) {
unload_plugins(gss_plugins);
+ }
nfs4_plugins = gss_plugins = NULL;
}
+ if (gss_methods)
+ conf_free_list(gss_methods);
+
return ret ? -ENOENT: 0;
}
--
2.20.1
On 5/8/19 9:35 AM, Steve Dickson wrote:
> Red Hat is now requiring covertity scans
> to be run against all RHEL 8 packages.
>
> These patches removed the majority of the
> resource leaks that were flagged by the scan.
>
> Most of the leaks were in return and error
> paths as well as some obvious problems like
> checking the wrong point to be NULL.
>
> There are still a few resources leaks
> and used_after_freed being flagged but
> I am thinking they false-positives
> because I just don't see the problem.
>
> I've tested these patches for a couple
> days and they seem stable... but whenever
> free()s are added... So is risk of freeing
> that is still being used. Plus they will
> get a good workout at the upcoming Bakeathon.
>
> Steve Dickson (19):
> Removed resource leaks from junction/path.c
> Removed resource leaks from nfs/exports.c
> Removed a resource leak from nfs/mydaemon.c
> Removed a resource leak from nfs/rpcmisc.c
> Removed a resource leak from nfs/svc_socket.c
> Removed bad frees from nfs/xcommon.c
> Removed resource leaks from nfs/xlog.c
> Removed resource leaks from nfsidmap/libnfsidmap.c
> Removed resource leaks from nfsidmap/static.c
> Removed a resource leak from nsm/file.c
> Removed resource leaks from systemd/rpc-pipefs-generator.c
> Removed resource leaks from blkmapd/device-discovery.c
> Removed resource leaks from gssd/krb5_util.c
> Removed a resource leak from mount/configfile.c
> Removed a resource leak from mount/nfsmount.c
> Removed a resource leak from mount/stropts.c
> Removed resource leaks from mountd/cache.c
> Removed a resource leak from mountd/fsloc.c
> Removed a resource leak from nfsdcltrack/sqlite.c
>
> support/junction/path.c | 6 +++++-
> support/nfs/exports.c | 2 ++
> support/nfs/mydaemon.c | 1 +
> support/nfs/rpcmisc.c | 1 +
> support/nfs/svc_socket.c | 1 +
> support/nfs/xcommon.c | 14 ++++++++++----
> support/nfs/xlog.c | 6 +++++-
> support/nfsidmap/libnfsidmap.c | 10 ++++++++--
> support/nfsidmap/static.c | 10 ++++++++++
> support/nsm/file.c | 1 +
> systemd/rpc-pipefs-generator.c | 10 ++++++++--
> utils/blkmapd/device-discovery.c | 22 +++++++++++++++++++++-
> utils/gssd/krb5_util.c | 9 ++++++++-
> utils/mount/configfile.c | 2 +-
> utils/mount/nfsmount.c | 1 +
> utils/mount/stropts.c | 5 ++++-
> utils/mountd/cache.c | 5 +++--
> utils/mountd/fsloc.c | 1 +
> utils/nfsdcltrack/sqlite.c | 2 ++
> 19 files changed, 93 insertions(+), 16 deletions(-)
>
Committed...
steved.