summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShyamsundarR <srangana@redhat.com>2018-07-30 14:09:14 -0400
committerAmar Tumballi <amarts@redhat.com>2018-08-03 08:37:15 +0000
commit60f1aeb08df80501caf6b17543592de02d61381f (patch)
treece4a8c5d163a9b4b20aeb8cd99041779455e1b67
parent9e96d646537fd060ca932291aaa0c4a3ce942b67 (diff)
coverity: Fix remaining SECURE_TEMP issues reported
Two pending SECURE_TEMP issues still exist in the coverity reports, these are fixed by this patch. In both instances (where functions actually seem to be duplicates of each other) the need was for a FILE * and not an fd. Applied the same pattern in both places as in other parts of the code where mkstemp was used and later a FILE * was created from the resulting fd for use. Coverity report: https://download.gluster.org/pub/gluster/ glusterfs/static-analysis/master/glusterfs-coverity/ 2018-07-30-4d3c62e7/html/ Issues numbered: 382, 383 (named SECURE_TEMP) Further added tmpfile to the blacklist, so that future code changes do not add the same, into symbol-check.sh. Also corrected shellcheck errors in symbol-check.sh as a result of updating the same. Updates: bz#789278 Change-Id: I1d572a16ca5b5df2f597aeaa5f454fad34c8296e Signed-off-by: ShyamsundarR <srangana@redhat.com>
-rw-r--r--api/src/glfs-mgmt.c32
-rw-r--r--glusterfsd/src/glusterfsd-messages.h3
-rw-r--r--glusterfsd/src/glusterfsd-mgmt.c32
-rwxr-xr-xtests/basic/symbol-check.sh33
4 files changed, 90 insertions, 10 deletions
diff --git a/api/src/glfs-mgmt.c b/api/src/glfs-mgmt.c
index 994f43c82f2..fac903b805a 100644
--- a/api/src/glfs-mgmt.c
+++ b/api/src/glfs-mgmt.c
@@ -582,6 +582,8 @@ glfs_mgmt_getspec_cbk (struct rpc_req *req, struct iovec *iov, int count,
struct glfs *fs = NULL;
dict_t *dict = NULL;
char *servers_list = NULL;
+ int tmp_fd = -1;
+ char template[] = "/tmp/gfapi.volfile.XXXXXX";
frame = myframe;
ctx = frame->this->ctx;
@@ -668,11 +670,28 @@ volfile:
goto out;
}
- tmpfp = tmpfile ();
- if (!tmpfp) {
- ret = -1;
- goto out;
- }
+ /* coverity[secure_temp] mkstemp uses 0600 as the mode and is safe */
+ tmp_fd = mkstemp (template);
+ if (-1 == tmp_fd) {
+ ret = -1;
+ goto out;
+ }
+
+ /* Calling unlink so that when the file is closed or program
+ * terminates the temporary file is deleted.
+ */
+ ret = sys_unlink (template);
+ if (ret < 0) {
+ gf_msg (frame->this->name, GF_LOG_INFO, 0, API_MSG_VOLFILE_INFO,
+ "Unable to delete file: %s", template);
+ ret = 0;
+ }
+
+ tmpfp = fdopen (tmp_fd, "w+b");
+ if (!tmpfp) {
+ ret = -1;
+ goto out;
+ }
fwrite (rsp.spec, size, 1, tmpfp);
fflush (tmpfp);
@@ -706,6 +725,7 @@ volfile:
ret = glfs_process_volfp (fs, tmpfp);
/* tmpfp closed */
tmpfp = NULL;
+ tmp_fd = -1;
if (ret)
goto out;
@@ -745,6 +765,8 @@ out:
if (tmpfp)
fclose (tmpfp);
+ else if (tmp_fd != -1)
+ sys_close (tmp_fd);
return 0;
}
diff --git a/glusterfsd/src/glusterfsd-messages.h b/glusterfsd/src/glusterfsd-messages.h
index 95fc79ef8d0..e7df714064a 100644
--- a/glusterfsd/src/glusterfsd-messages.h
+++ b/glusterfsd/src/glusterfsd-messages.h
@@ -61,7 +61,8 @@ GLFS_MSGID(GLUSTERFSD,
glusterfsd_msg_35,
glusterfsd_msg_36,
glusterfsd_msg_37,
- glusterfsd_msg_38
+ glusterfsd_msg_38,
+ glusterfsd_msg_39
);
#endif /* !_GLUSTERFSD_MESSAGES_H_ */
diff --git a/glusterfsd/src/glusterfsd-mgmt.c b/glusterfsd/src/glusterfsd-mgmt.c
index e954c1f3d33..bf56bc0abfa 100644
--- a/glusterfsd/src/glusterfsd-mgmt.c
+++ b/glusterfsd/src/glusterfsd-mgmt.c
@@ -1901,6 +1901,8 @@ mgmt_getspec_cbk (struct rpc_req *req, struct iovec *iov, int count,
char sha256_hash[SHA256_DIGEST_LENGTH] = {0, };
dict_t *dict = NULL;
char *servers_list = NULL;
+ int tmp_fd = -1;
+ char template[] = "/tmp/glfs.volfile.XXXXXX";
frame = myframe;
ctx = frame->this->ctx;
@@ -1990,7 +1992,32 @@ volfile:
}
}
- tmpfp = tmpfile ();
+ /* coverity[secure_temp] mkstemp uses 0600 as the mode and is
+ * safe
+ */
+ tmp_fd = mkstemp (template);
+ if (-1 == tmp_fd) {
+ gf_msg (frame->this->name, GF_LOG_ERROR, 0,
+ glusterfsd_msg_39,
+ "Unable to create temporary file: %s",
+ template);
+ ret = -1;
+ goto out;
+ }
+
+ /* Calling unlink so that when the file is closed or program
+ * terminates the temporary file is deleted.
+ */
+ ret = sys_unlink (template);
+ if (ret < 0) {
+ gf_msg (frame->this->name, GF_LOG_INFO, 0,
+ glusterfsd_msg_39,
+ "Unable to delete temporary file: %s",
+ template);
+ ret = 0;
+ }
+
+ tmpfp = fdopen (tmp_fd, "w+b");
if (!tmpfp) {
ret = -1;
goto out;
@@ -2036,6 +2063,7 @@ volfile:
ret = glusterfs_process_volfp (ctx, tmpfp);
/* tmpfp closed */
tmpfp = NULL;
+ tmp_fd = -1;
if (ret)
goto out;
@@ -2103,6 +2131,8 @@ out:
if (tmpfp)
fclose (tmpfp);
+ else if (tmp_fd != -1)
+ sys_close (tmp_fd);
return 0;
}
diff --git a/tests/basic/symbol-check.sh b/tests/basic/symbol-check.sh
index f84d591facb..0f8243ca731 100755
--- a/tests/basic/symbol-check.sh
+++ b/tests/basic/symbol-check.sh
@@ -13,6 +13,8 @@ syscalls32=$'creat\nfallocate\nftruncate\n__fxstat\n__fxstatat\n\
lseek\n__lxstat\nopenat\nreaddir\nstatvfs\ntruncate\nstat\n\
preadv\npwritev\npread\npwrite'
+glibccalls=$'tmpfile'
+
exclude_files=$'/libglusterfs/src/.libs/libglusterfs_la-syscall.o\n\
/libglusterfs/src/.libs/libglusterfs_la-gen_uuid.o\n\
/contrib/fuse-util/fusermount.o\n\
@@ -33,13 +35,14 @@ function main()
done
local retval=0
- local t=$(nm ${1} | grep " U " | sed -e "s/ //g" -e "s/ U //g")
+ local t
+ t=$(nm "${1}" | grep " U " | sed -e "s/ //g" -e "s/ U //g")
for symy in ${t}; do
for symx in ${syscalls}; do
- if [[ ${symx} = ${symy} ]]; then
+ if [[ ${symx} = "${symy}" ]]; then
case ${symx} in
"creat64") sym="creat";;
@@ -70,12 +73,36 @@ function main()
for symx in ${syscalls32}; do
- if [[ ${symx} = ${symy} ]]; then
+ if [[ ${symx} = "${symy}" ]]; then
echo "${1} was not compiled with -D_FILE_OFFSET_BITS=64" >&2
retval=1
fi
done
+
+ symy_glibc=$(echo "${symy}" | sed -e "s/@@GLIBC.*//g")
+ # Eliminate false positives, check if we have a GLIBC symbol in 'y'
+ if [[ ${symy} != "${symy_glibc}" ]]; then
+ for symx in ${glibccalls}; do
+
+ if [[ ${symx} = "${symy_glibc}" ]]; then
+
+ case ${symx} in
+ "tmpfile") alt="mkstemp";;
+ *) alt="none";;
+ esac
+
+ if [[ ${alt} = "none" ]]; then
+ echo "${1} should not call ${symy_glibc}";
+ else
+ echo "${1} should use ${alt} instead of ${symy_glibc}" >&2;
+ fi
+
+ retval=1
+ fi
+ done
+ fi
+
done
if [ ${retval} = 1 ]; then