2020-07-10 20:38:02

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils PATCH 0/5] nfsdcld: Fix a number of Coverity Scan

These patches fix some errors in nfsdcld flagged by the Coverity scan
software.

Scott Mayhew (5):
nfsdcld: Fix a few Coverity Scan RESOURCE_LEAK errors
nfsdcld: Fix a few Coverity Scan TOCTOU errors
nfsdcld: Fix a few Coverity Scan STRING_NULL errors
nfsdcld: Fix a few Coverity Scan CLANG_WARNING errors
nfsdcld: Fix a few Coverity Scan CHECKED_RETURN errors.

utils/nfsdcld/legacy.c | 38 ++++++++++++--------------------------
utils/nfsdcld/nfsdcld.c | 7 +++++--
utils/nfsdcld/sqlite.c | 5 +++--
3 files changed, 20 insertions(+), 30 deletions(-)

--
2.25.4


2020-07-10 20:38:20

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils PATCH 1/5] nfsdcld: Fix a few Coverity Scan RESOURCE_LEAK errors

Signed-off-by: Scott Mayhew <[email protected]>
---
utils/nfsdcld/legacy.c | 2 ++
utils/nfsdcld/nfsdcld.c | 1 +
utils/nfsdcld/sqlite.c | 2 +-
3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/utils/nfsdcld/legacy.c b/utils/nfsdcld/legacy.c
index 3c6bea6..b8ea4ff 100644
--- a/utils/nfsdcld/legacy.c
+++ b/utils/nfsdcld/legacy.c
@@ -60,6 +60,7 @@ legacy_load_clients_from_recdir(int *num_records)
}
if (read(fd, recdirname, PATH_MAX) < 0) {
xlog(D_GENERAL, "Unable to read from %s: %m", NFSD_RECDIR_FILE);
+ close(fd);
return;
}
close(fd);
@@ -135,6 +136,7 @@ legacy_clear_recdir(void)
}
if (read(fd, recdirname, PATH_MAX) < 0) {
xlog(D_GENERAL, "Unable to read from %s: %m", NFSD_RECDIR_FILE);
+ close(fd);
return;
}
close(fd);
diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
index be65562..bb8e365 100644
--- a/utils/nfsdcld/nfsdcld.c
+++ b/utils/nfsdcld/nfsdcld.c
@@ -331,6 +331,7 @@ cld_check_grace_period(void)
if (read(fd, &c, 1) < 0) {
xlog(L_WARNING, "Unable to read from %s: %m",
NFSD_END_GRACE_FILE);
+ close(fd);
return 1;
}
close(fd);
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index 6666c86..e61e67c 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -380,7 +380,7 @@ sqlite_maindb_init_v4(void)
&err);
if (ret != SQLITE_OK) {
xlog(L_ERROR, "Unable to begin transaction: %s", err);
- return ret;
+ goto out;
}

/*
--
2.25.4

2020-07-10 20:38:38

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils PATCH 2/5] nfsdcld: Fix a few Coverity Scan TOCTOU errors

Calling stat() on recdirname so that we can see if it's a directory is
unnecessary anyways, since opendir() will report an error if it's not.

Signed-off-by: Scott Mayhew <[email protected]>
---
utils/nfsdcld/legacy.c | 20 --------------------
1 file changed, 20 deletions(-)

diff --git a/utils/nfsdcld/legacy.c b/utils/nfsdcld/legacy.c
index b8ea4ff..1fb74d4 100644
--- a/utils/nfsdcld/legacy.c
+++ b/utils/nfsdcld/legacy.c
@@ -50,7 +50,6 @@ legacy_load_clients_from_recdir(int *num_records)
struct dirent *entry;
char recdirname[PATH_MAX];
char buf[NFS4_OPAQUE_LIMIT];
- struct stat st;
char *nl;

fd = open(NFSD_RECDIR_FILE, O_RDONLY);
@@ -69,15 +68,6 @@ legacy_load_clients_from_recdir(int *num_records)
if (!nl)
return;
*nl = '\0';
- if (stat(recdirname, &st) < 0) {
- xlog(D_GENERAL, "Unable to stat %s: %d", recdirname, errno);
- return;
- }
- if (!S_ISDIR(st.st_mode)) {
- xlog(D_GENERAL, "%s is not a directory: mode=0%o", recdirname
- , st.st_mode);
- return;
- }
v4recovery = opendir(recdirname);
if (!v4recovery)
return;
@@ -126,7 +116,6 @@ legacy_clear_recdir(void)
struct dirent *entry;
char recdirname[PATH_MAX];
char dirname[PATH_MAX];
- struct stat st;
char *nl;

fd = open(NFSD_RECDIR_FILE, O_RDONLY);
@@ -145,15 +134,6 @@ legacy_clear_recdir(void)
if (!nl)
return;
*nl = '\0';
- if (stat(recdirname, &st) < 0) {
- xlog(D_GENERAL, "Unable to stat %s: %d", recdirname, errno);
- return;
- }
- if (!S_ISDIR(st.st_mode)) {
- xlog(D_GENERAL, "%s is not a directory: mode=0%o", recdirname
- , st.st_mode);
- return;
- }
v4recovery = opendir(recdirname);
if (!v4recovery)
return;
--
2.25.4

2020-07-10 20:38:42

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils PATCH 4/5] nfsdcld: Fix a few Coverity Scan CLANG_WARNING errors

Signed-off-by: Scott Mayhew <[email protected]>
---
utils/nfsdcld/nfsdcld.c | 6 ++++--
utils/nfsdcld/sqlite.c | 3 ++-
2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
index bb8e365..9b6d2c3 100644
--- a/utils/nfsdcld/nfsdcld.c
+++ b/utils/nfsdcld/nfsdcld.c
@@ -252,11 +252,12 @@ cld_inotify_setup(void)
xlog_err("%s: inotify_add_watch failed: %m", __func__);
ret = -errno;
goto out_err;
- }
+ } else
+ ret = 0;

out_free:
free(dirc);
- return 0;
+ return ret;
out_err:
close(inotify_fd);
goto out_free;
@@ -796,6 +797,7 @@ main(int argc, char **argv)
break;
default:
usage(progname);
+ free(progname);
return 0;
}
}
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index e61e67c..ef11a54 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -831,7 +831,6 @@ sqlite_prepare_dbh(const char *topdir)
switch (ret) {
case CLD_SQLITE_LATEST_SCHEMA_VERSION:
/* DB is already set up. Do nothing */
- ret = 0;
break;
case 3:
/* Old DB -- update to new schema */
@@ -868,6 +867,8 @@ sqlite_prepare_dbh(const char *topdir)
}

ret = sqlite_startup_query_grace();
+ if (ret)
+ goto out_close;

ret = sqlite_query_first_time(&first_time);
if (ret)
--
2.25.4

2020-07-10 20:39:00

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils PATCH 3/5] nfsdcld: Fix a few Coverity Scan STRING_NULL errors

Signed-off-by: Scott Mayhew <[email protected]>
---
utils/nfsdcld/legacy.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/utils/nfsdcld/legacy.c b/utils/nfsdcld/legacy.c
index 1fb74d4..9e9f758 100644
--- a/utils/nfsdcld/legacy.c
+++ b/utils/nfsdcld/legacy.c
@@ -48,7 +48,7 @@ legacy_load_clients_from_recdir(int *num_records)
int fd;
DIR *v4recovery;
struct dirent *entry;
- char recdirname[PATH_MAX];
+ char recdirname[PATH_MAX+1];
char buf[NFS4_OPAQUE_LIMIT];
char *nl;

@@ -64,6 +64,7 @@ legacy_load_clients_from_recdir(int *num_records)
}
close(fd);
/* the output from the proc file isn't null-terminated */
+ recdirname[PATH_MAX] = '\0';
nl = strchr(recdirname, '\n');
if (!nl)
return;
@@ -114,7 +115,7 @@ legacy_clear_recdir(void)
int fd;
DIR *v4recovery;
struct dirent *entry;
- char recdirname[PATH_MAX];
+ char recdirname[PATH_MAX+1];
char dirname[PATH_MAX];
char *nl;

@@ -130,6 +131,7 @@ legacy_clear_recdir(void)
}
close(fd);
/* the output from the proc file isn't null-terminated */
+ recdirname[PATH_MAX] = '\0';
nl = strchr(recdirname, '\n');
if (!nl)
return;
--
2.25.4

2020-07-10 20:39:18

by Scott Mayhew

[permalink] [raw]
Subject: [nfs-utils PATCH 5/5] nfsdcld: Fix a few Coverity Scan CHECKED_RETURN errors.

Signed-off-by: Scott Mayhew <[email protected]>
---
utils/nfsdcld/legacy.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/utils/nfsdcld/legacy.c b/utils/nfsdcld/legacy.c
index 9e9f758..b89374c 100644
--- a/utils/nfsdcld/legacy.c
+++ b/utils/nfsdcld/legacy.c
@@ -51,18 +51,19 @@ legacy_load_clients_from_recdir(int *num_records)
char recdirname[PATH_MAX+1];
char buf[NFS4_OPAQUE_LIMIT];
char *nl;
+ ssize_t n;

fd = open(NFSD_RECDIR_FILE, O_RDONLY);
if (fd < 0) {
xlog(D_GENERAL, "Unable to open %s: %m", NFSD_RECDIR_FILE);
return;
}
- if (read(fd, recdirname, PATH_MAX) < 0) {
+ n = read(fd, recdirname, PATH_MAX);
+ close(fd);
+ if (n < 0) {
xlog(D_GENERAL, "Unable to read from %s: %m", NFSD_RECDIR_FILE);
- close(fd);
return;
}
- close(fd);
/* the output from the proc file isn't null-terminated */
recdirname[PATH_MAX] = '\0';
nl = strchr(recdirname, '\n');
@@ -118,18 +119,19 @@ legacy_clear_recdir(void)
char recdirname[PATH_MAX+1];
char dirname[PATH_MAX];
char *nl;
+ ssize_t n;

fd = open(NFSD_RECDIR_FILE, O_RDONLY);
if (fd < 0) {
xlog(D_GENERAL, "Unable to open %s: %m", NFSD_RECDIR_FILE);
return;
}
- if (read(fd, recdirname, PATH_MAX) < 0) {
+ n = read(fd, recdirname, PATH_MAX);
+ close(fd);
+ if (n < 0) {
xlog(D_GENERAL, "Unable to read from %s: %m", NFSD_RECDIR_FILE);
- close(fd);
return;
}
- close(fd);
/* the output from the proc file isn't null-terminated */
recdirname[PATH_MAX] = '\0';
nl = strchr(recdirname, '\n');
--
2.25.4

2020-07-17 13:55:03

by Steve Dickson

[permalink] [raw]
Subject: Re: [nfs-utils PATCH 0/5] nfsdcld: Fix a number of Coverity Scan



On 7/10/20 4:36 PM, Scott Mayhew wrote:
> These patches fix some errors in nfsdcld flagged by the Coverity scan
> software.
>
> Scott Mayhew (5):
> nfsdcld: Fix a few Coverity Scan RESOURCE_LEAK errors
> nfsdcld: Fix a few Coverity Scan TOCTOU errors
> nfsdcld: Fix a few Coverity Scan STRING_NULL errors
> nfsdcld: Fix a few Coverity Scan CLANG_WARNING errors
> nfsdcld: Fix a few Coverity Scan CHECKED_RETURN errors.
>
> utils/nfsdcld/legacy.c | 38 ++++++++++++--------------------------
> utils/nfsdcld/nfsdcld.c | 7 +++++--
> utils/nfsdcld/sqlite.c | 5 +++--
> 3 files changed, 20 insertions(+), 30 deletions(-)
>
Committed... (tag: nfs-utils-2-5-2-rc2)

steved.