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

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

[Scott Mayhew]

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

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

[Scott Mayhew]

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 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

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

[Scott Mayhew]

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 <smayhew@redhat.com>
---
 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

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

[Scott Mayhew]

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 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

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

[Scott Mayhew]

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 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

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

[Scott Mayhew]

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 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

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

[Steve Dickson]

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.