From: Jeffrey Hutzelman Date: Sun, 16 Jun 2013 19:28:03 +0000 (-0400) Subject: Fix unchecked return values X-Git-Tag: upstream/1.6.21^2~48 X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=9bf43d5ce036fd0e3f0fa61a6e3cbae1c925cc28;p=packages%2Fo%2Fopenafs.git Fix unchecked return values This change fixes numerous places where the return values of various system calls and standard library routines are not checked. In particular, this fixes occurrances called out when building on Ubuntu 12.10, with gcc 4.7.2 and eglibc 2.15-0ubuntu20.1, when the possible failure is one we actually do (or should) care about. This change does not consider calls where the failure is one we deliberately choose to ignore. Reviewed-on: http://gerrit.openafs.org/9979 Tested-by: BuildBot Reviewed-by: Jeffrey Altman (cherry picked from commit 720363fa9bf7cfbebdc485104b74ca6bac1895f6) Change-Id: I9e72a68a66d751139f7129b9b177ba18389c10a2 Reviewed-on: https://gerrit.openafs.org/12521 Tested-by: BuildBot Reviewed-by: Mark Vitale Reviewed-by: Stephan Wiesand --- diff --git a/src/auth/cellconfig.c b/src/auth/cellconfig.c index ec95251fa..5e6e7f095 100644 --- a/src/auth/cellconfig.c +++ b/src/auth/cellconfig.c @@ -460,7 +460,7 @@ afsconf_Open(const char *adir) /* The "AFSCONF" environment (or contents of "/.AFSCONF") will be typically set to something like "/afs//common/etc" where, by convention, the default files for "ThisCell" and "CellServDB" will reside; note that a major drawback is that a given afs client on that cell may NOT contain the same contents... */ char *home_dir; afsconf_FILE *fp; - size_t len; + size_t len = 0; if (!(home_dir = getenv("HOME"))) { /* Our last chance is the "/.AFSCONF" file */ @@ -468,8 +468,6 @@ afsconf_Open(const char *adir) if (fp == 0) goto fail; - fgets(afs_confdir, 128, fp); - fclose(fp); } else { char *pathname = NULL; @@ -486,10 +484,10 @@ afsconf_Open(const char *adir) if (fp == 0) goto fail; } - fgets(afs_confdir, 128, fp); - fclose(fp); } - len = strlen(afs_confdir); + if (fgets(afs_confdir, 128, fp) != NULL) + len = strlen(afs_confdir); + fclose(fp); if (len == 0) goto fail; diff --git a/src/bozo/bosserver.c b/src/bozo/bosserver.c index 179475435..b596c6a5a 100644 --- a/src/bozo/bosserver.c +++ b/src/bozo/bosserver.c @@ -169,32 +169,46 @@ CreateDirs(const char *coredir) (!strncmp (AFSDIR_USR_DIRPATH, AFSDIR_SERVER_BIN_DIRPATH, strlen(AFSDIR_USR_DIRPATH)))) { - MakeDir(AFSDIR_USR_DIRPATH); + if (MakeDir(AFSDIR_USR_DIRPATH)) + return errno; } if (!strncmp (AFSDIR_SERVER_AFS_DIRPATH, AFSDIR_SERVER_BIN_DIRPATH, strlen(AFSDIR_SERVER_AFS_DIRPATH))) { - MakeDir(AFSDIR_SERVER_AFS_DIRPATH); + if (MakeDir(AFSDIR_SERVER_AFS_DIRPATH)) + return errno; } - MakeDir(AFSDIR_SERVER_BIN_DIRPATH); - MakeDir(AFSDIR_SERVER_ETC_DIRPATH); - MakeDir(AFSDIR_SERVER_LOCAL_DIRPATH); - MakeDir(AFSDIR_SERVER_DB_DIRPATH); - MakeDir(AFSDIR_SERVER_LOGS_DIRPATH); + if (MakeDir(AFSDIR_SERVER_BIN_DIRPATH)) + return errno; + if (MakeDir(AFSDIR_SERVER_ETC_DIRPATH)) + return errno; + if (MakeDir(AFSDIR_SERVER_LOCAL_DIRPATH)) + return errno; + if (MakeDir(AFSDIR_SERVER_DB_DIRPATH)) + return errno; + if (MakeDir(AFSDIR_SERVER_LOGS_DIRPATH)) + return errno; #ifndef AFS_NT40_ENV if (!strncmp (AFSDIR_CLIENT_VICE_DIRPATH, AFSDIR_CLIENT_ETC_DIRPATH, strlen(AFSDIR_CLIENT_VICE_DIRPATH))) { - MakeDir(AFSDIR_CLIENT_VICE_DIRPATH); + if (MakeDir(AFSDIR_CLIENT_VICE_DIRPATH)) + return errno; } - MakeDir(AFSDIR_CLIENT_ETC_DIRPATH); + if (MakeDir(AFSDIR_CLIENT_ETC_DIRPATH)) + return errno; - symlink(AFSDIR_SERVER_THISCELL_FILEPATH, AFSDIR_CLIENT_THISCELL_FILEPATH); - symlink(AFSDIR_SERVER_CELLSERVDB_FILEPATH, - AFSDIR_CLIENT_CELLSERVDB_FILEPATH); + if (symlink(AFSDIR_SERVER_THISCELL_FILEPATH, + AFSDIR_CLIENT_THISCELL_FILEPATH)) + return errno; + if (symlink(AFSDIR_SERVER_CELLSERVDB_FILEPATH, + AFSDIR_CLIENT_CELLSERVDB_FILEPATH)) + return errno; #endif /* AFS_NT40_ENV */ - if (coredir) - MakeDir(coredir); + if (coredir) { + if (MakeDir(coredir)) + return errno; + } return 0; } @@ -892,21 +906,33 @@ main(int argc, char **argv, char **envp) bnode_Register("cron", &cronbnode_ops, 2); /* create useful dirs */ - CreateDirs(DoCore); + i = CreateDirs(DoCore); + if (i) { + printf("bosserver: could not set up directories, code %d\n", i); + exit(1); + } /* chdir to AFS log directory */ if (DoCore) - chdir(DoCore); + i = chdir(DoCore); else - chdir(AFSDIR_SERVER_LOGS_DIRPATH); + i = chdir(AFSDIR_SERVER_LOGS_DIRPATH); + + if (i) { + printf("bosserver: could not change to %s, code %d\n", + DoCore ? DoCore : AFSDIR_SERVER_LOGS_DIRPATH, errno); + exit(1); + } /* go into the background and remove our controlling tty, close open file desriptors */ #ifndef AFS_NT40_ENV - if (!nofork) - daemon(1, 0); + if (!nofork) { + if (daemon(1, 0)) + printf("bosserver: warning - daemon() returned code %d\n", errno); + } #endif /* ! AFS_NT40_ENV */ if ((!DoSyslog) diff --git a/src/budb/db_text.c b/src/budb/db_text.c index 083c3cf58..5e1bbbee1 100644 --- a/src/budb/db_text.c +++ b/src/budb/db_text.c @@ -466,23 +466,29 @@ saveTextToFile(struct ubik_trans *ut, struct textBlock *tbPtr) afs_int32 blockAddr; struct block block; char filename[128]; - afs_int32 size, chunkSize; + afs_int32 size, totalSize, chunkSize; int fid; sprintf(filename, "%s/%s", gettmpdir(), "dbg_XXXXXX"); fid = mkstemp(filename); - size = ntohl(tbPtr->size); + totalSize = size = ntohl(tbPtr->size); blockAddr = ntohl(tbPtr->textAddr); while (size) { chunkSize = MIN(BLOCK_DATA_SIZE, size); dbread(ut, blockAddr, (char *)&block, sizeof(block)); - write(fid, &block.a[0], chunkSize); + if (write(fid, &block.a[0], chunkSize) < 0) + break; blockAddr = ntohl(block.h.next); size -= chunkSize; } close(fid); - printf("wrote debug file %s\n", filename); + if (size) { + printf("Wrote partial debug file (%ld bytes out of %ld)\n", + (long)(totalSize - size), (long)totalSize); + } else { + printf("wrote debug file %s\n", filename); + } } diff --git a/src/kauth/kkids.c b/src/kauth/kkids.c index 1d755274d..f54dc5408 100644 --- a/src/kauth/kkids.c +++ b/src/kauth/kkids.c @@ -396,8 +396,10 @@ init_child(char *myname) * reads from the latter, the child reads from the former, and * writes to the latter. */ - pipe(pipe1); - pipe(pipe2); + if (pipe(pipe1) || pipe(pipe2)) { + using_child = 0; + return 0; + } /* fork a child */ pid = fork(); @@ -438,7 +440,8 @@ password_bad(char *pw) if (using_child) { fprintf(childin, "%s\n", pw); fflush(childin); - fscanf(childout, "%d", &rc); + if (fscanf(childout, "%d", &rc) < 1) + rc = -1; } return (rc); diff --git a/src/lwp/lwp.c b/src/lwp/lwp.c index 35eb8cf3e..463a61a5a 100644 --- a/src/lwp/lwp.c +++ b/src/lwp/lwp.c @@ -996,11 +996,15 @@ Overflow_Complain(void) currenttime = time(0); timeStamp = ctime(¤ttime); timeStamp[24] = 0; - write(2, timeStamp, strlen(timeStamp)); - - write(2, msg1, strlen(msg1)); - write(2, lwp_cpptr->name, strlen(lwp_cpptr->name)); - write(2, msg2, strlen(msg2)); + if (write(2, timeStamp, strlen(timeStamp)) < 0) + return; + + if (write(2, msg1, strlen(msg1)) < 0) + return; + if (write(2, lwp_cpptr->name, strlen(lwp_cpptr->name)) < 0) + return; + if (write(2, msg2, strlen(msg2)) < 0) + return; } static void diff --git a/src/ptserver/pt_util.c b/src/ptserver/pt_util.c index b1c6fe41a..f9b2c259b 100644 --- a/src/ptserver/pt_util.c +++ b/src/ptserver/pt_util.c @@ -401,7 +401,11 @@ static int display_entry(int offset) { lseek(dbase_fd, offset + HDRSIZE, L_SET); - read(dbase_fd, &pre, sizeof(struct prentry)); + if (read(dbase_fd, &pre, sizeof(struct prentry)) < 0) { + fprintf(stderr, "pt_util: error reading entry %d: %s\n", + offset, strerror(errno)); + exit(1); + } fix_pre(&pre); @@ -500,7 +504,11 @@ display_group(int id) offset = pre.next; while (offset) { lseek(dbase_fd, offset + HDRSIZE, L_SET); - read(dbase_fd, &prco, sizeof(struct contentry)); + if (read(dbase_fd, &prco, sizeof(struct contentry)) < 0) { + fprintf(stderr, "pt_util: read i/o error: %s\n", + strerror(errno)); + exit(1); + } prco.next = ntohl(prco.next); for (i = 0; i < COSIZE; i++) { prco.entries[i] = ntohl(prco.entries[i]); diff --git a/src/ptserver/ptubik.c b/src/ptserver/ptubik.c index 00bd944cf..c77131a96 100644 --- a/src/ptserver/ptubik.c +++ b/src/ptserver/ptubik.c @@ -12,6 +12,7 @@ #endif #include #include +#include #include #define UBIK_INTERNALS @@ -38,6 +39,7 @@ ubik_BeginTrans(struct ubik_dbase *dbase, afs_int32 transMode, { static int init = 0; struct ubik_hdr thdr; + ssize_t count; if (!init) { memset(&thdr, 0, sizeof(thdr)); @@ -45,9 +47,15 @@ ubik_BeginTrans(struct ubik_dbase *dbase, afs_int32 transMode, thdr.version.counter = htonl(0); thdr.magic = htonl(UBIK_MAGIC); thdr.size = htons(HDRSIZE); - lseek(dbase_fd, 0, 0); - write(dbase_fd, &thdr, sizeof(thdr)); - fsync(dbase_fd); + if (lseek(dbase_fd, 0, 0) == (off_t)-1) + return errno; + count = write(dbase_fd, &thdr, sizeof(thdr)); + if (count < 0) + return errno; + else if (count != sizeof(thdr)) + return UIOERROR; + if (fsync(dbase_fd)) + return errno; init = 1; } return (0); diff --git a/src/sys/rmtsysc.c b/src/sys/rmtsysc.c index 6901febb0..490626111 100644 --- a/src/sys/rmtsysc.c +++ b/src/sys/rmtsysc.c @@ -74,7 +74,7 @@ GetAfsServerAddr(char *syscall) if (!(afs_server = getenv("AFSSERVER"))) { char *home_dir; FILE *fp; - int len; + int len = 0; if (!(home_dir = getenv("HOME"))) { /* Our last chance is the "/.AFSSERVER" file */ @@ -82,8 +82,6 @@ GetAfsServerAddr(char *syscall) if (fp == 0) { return 0; } - fgets(server_name, 128, fp); - fclose(fp); } else { char *pathname; @@ -100,10 +98,10 @@ GetAfsServerAddr(char *syscall) return 0; } } - fgets(server_name, 128, fp); - fclose(fp); } - len = strlen(server_name); + if (fgets(server_name, 128, fp) != NULL) + len = strlen(server_name); + fclose(fp); if (len == 0) { return 0; } diff --git a/src/tviced/state_analyzer.c b/src/tviced/state_analyzer.c index 5e8039494..1d050c21e 100644 --- a/src/tviced/state_analyzer.c +++ b/src/tviced/state_analyzer.c @@ -368,7 +368,8 @@ prompt(void) fprintf(stderr, "prompt state broken; aborting\n"); return; } - fgets(input, 256, stdin); + if (fgets(input, 256, stdin) == NULL) + return; if (!strcmp(input, "")) { /* repeat last command */ diff --git a/src/venus/test/fulltest.c b/src/venus/test/fulltest.c index dda9d9a64..e3b6d9beb 100644 --- a/src/venus/test/fulltest.c +++ b/src/venus/test/fulltest.c @@ -233,7 +233,7 @@ main(int argc, char **argv) perror("open ronly"); return -1; } - fchown(fd1, 1, -1); /* don't check error code, may fail on Ultrix */ + code = fchown(fd1, 1, -1); /* don't check error code, may fail on Ultrix */ code = write(fd1, "test", 4); if (code != 4) { printf("rotest short read (%d)\n", code); @@ -296,7 +296,10 @@ main(int argc, char **argv) } /* now finish up */ - chdir(".."); + if (chdir("..") < 0) { + perror("chdir .."); + return -1; + } rmdir(dirName); printf("Test completed successfully.\n"); return 0; diff --git a/src/venus/up.c b/src/venus/up.c index 6a62ca995..4ee28849f 100644 --- a/src/venus/up.c +++ b/src/venus/up.c @@ -210,8 +210,10 @@ MakeParent(char *file, afs_int32 owner) fflush(stdout); } - mkdir(parent, 0777); - chown(parent, owner, -1); + if (mkdir(parent, 0777)) + return (0); + if (chown(parent, owner, -1)) + return (0); } return (1); } /*MakeParent */ diff --git a/src/viced/afsfileprocs.c b/src/viced/afsfileprocs.c index 31add15b6..e145d67be 100644 --- a/src/viced/afsfileprocs.c +++ b/src/viced/afsfileprocs.c @@ -7686,13 +7686,17 @@ StoreData_RXStyle(Volume * volptr, Vnode * targetptr, struct AFSFid * Fid, (afs_uintmax_t) Pos, (afs_uintmax_t) DataLength, (afs_uintmax_t) FileLength, (afs_uintmax_t) Length)); - /* truncate the file iff it needs it (ftruncate is slow even when its a noop) */ - if (FileLength < DataLength) - FDH_TRUNC(fdP, FileLength); bytesTransfered = 0; #ifndef HAVE_PIOV tbuffer = AllocSendBuffer(); #endif /* HAVE_PIOV */ + /* truncate the file iff it needs it (ftruncate is slow even when its a noop) */ + if (FileLength < DataLength) { + errorCode = FDH_TRUNC(fdP, FileLength); + if (errorCode) + goto done; + } + /* if length == 0, the loop below isn't going to do anything, including * extend the length of the inode, which it must do, since the file system * assumes that the inode length == vnode's file length. So, we extend diff --git a/src/viced/viced.c b/src/viced/viced.c index 966571ddb..51bd0aa14 100644 --- a/src/viced/viced.c +++ b/src/viced/viced.c @@ -414,18 +414,18 @@ char adminName[MAXADMINNAME]; static void CheckAdminName(void) { - int fd = 0; + int fd = -1; struct afs_stat status; if ((afs_stat("/AdminName", &status)) || /* if file does not exist */ (status.st_size <= 0) || /* or it is too short */ (status.st_size >= (MAXADMINNAME)) || /* or it is too long */ - !(fd = afs_open("/AdminName", O_RDONLY, 0))) { /* or the open fails */ + (fd = afs_open("/AdminName", O_RDONLY, 0)) < 0 || /* or open fails */ + read(fd, adminName, status.st_size) != status.st_size) { /* or read */ + strcpy(adminName, "System:Administrators"); /* use the default name */ - } else { - (void)read(fd, adminName, status.st_size); /* use name from the file */ } - if (fd) + if (fd >= 0) close(fd); /* close fd if it was opened */ } /*CheckAdminName */ diff --git a/src/vlserver/cnvldb.c b/src/vlserver/cnvldb.c index 379d4ba74..f56171ee6 100644 --- a/src/vlserver/cnvldb.c +++ b/src/vlserver/cnvldb.c @@ -76,6 +76,8 @@ static int handleit(struct cmd_syndesc *as, void *arock) { int w, old, new, rc, dump = 0, fromv = 0; + ssize_t count; + char ubik[80]; /* space for some ubik header */ union { struct vlheader_1 header1; @@ -105,8 +107,18 @@ handleit(struct cmd_syndesc *as, void *arock) } /* Read the version */ - lseek(old, 64, L_SET); - read(old, &fromv, sizeof(int)); + if (lseek(old, 64, L_SET) == (off_t)-1) { + perror(pn); + exit(-1); + } + count = read(old, &fromv, sizeof(int)); + if (count < 0) { + perror(pn); + exit(-1); + } else if (count != sizeof(int)) { + fprintf(stderr, "%s: Premature EOF reading database version.\n", pn); + exit(-1); + } fromv = ntohl(fromv); if ((fromv < 1) || (fromv > 4)) { fprintf(stderr, "%s", pn); @@ -115,8 +127,18 @@ handleit(struct cmd_syndesc *as, void *arock) } /* Sequentially read the database converting the entries as we go */ - lseek(old, 0, L_SET); - read(old, ubik, 64); + if (lseek(old, 0, L_SET) == (off_t)-1) { + perror(pn); + exit(-1); + } + count = read(old, ubik, 64); + if (count < 0) { + perror(pn); + exit(-1); + } else if (count != 64) { + fprintf(stderr, "%s: Premature EOF reading database header.\n", pn); + exit(-1); + } readheader(old, fromv, &oldheader); if (fromv == 1) { dbsize = ntohl(oldheader.header1.vital_header.eofPtr); diff --git a/src/vol/namei_ops.c b/src/vol/namei_ops.c index de6dc889a..21f9e9e5f 100644 --- a/src/vol/namei_ops.c +++ b/src/vol/namei_ops.c @@ -378,7 +378,7 @@ int namei_ViceREADME(char *partition) { char filename[32]; - int fd; + int fd, len, e = 0; /* Create the inode directory if we're starting for the first time */ (void)afs_snprintf(filename, sizeof filename, "%s" OS_DIRSEP "%s", partition, @@ -389,8 +389,13 @@ namei_ViceREADME(char *partition) partition, INODEDIR); fd = afs_open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0444); if (fd >= 0) { - (void)write(fd, VICE_README, strlen(VICE_README)); + len = strlen(VICE_README); + if (write(fd, VICE_README, len) != len) + e = errno; close(fd); + if (e) + errno = e; + } return (errno); } @@ -1614,7 +1619,10 @@ namei_GetLinkCount(FdHandle_t * h, Inode ino, int lockit, int fixup, int nowrite NAMEI_GLC_UNLOCK; goto bad_getLinkByte; } - FDH_TRUNC(h, offset+sizeof(row)); + if (FDH_TRUNC(h, offset+sizeof(row))) { + NAMEI_GLC_UNLOCK; + goto bad_getLinkByte; + } row = 1 << index; rc = FDH_PWRITE(h, (char *)&row, sizeof(row), offset); NAMEI_GLC_UNLOCK; @@ -3214,7 +3222,11 @@ namei_ConvertROtoRWvolume(char *pname, afs_uint32 volumeId) #ifdef AFS_NT40_ENV MoveFileEx(n.n_path, newpath, MOVEFILE_WRITE_THROUGH); #else - link(newpath, n.n_path); + if (link(newpath, n.n_path)) { + Log("1 namei_ConvertROtoRWvolume: could not move SmallIndex file: %s\n", n.n_path); + code = -1; + goto done; + } OS_UNLINK(newpath); #endif @@ -3233,7 +3245,11 @@ namei_ConvertROtoRWvolume(char *pname, afs_uint32 volumeId) #ifdef AFS_NT40_ENV MoveFileEx(n.n_path, newpath, MOVEFILE_WRITE_THROUGH); #else - link(newpath, n.n_path); + if (link(newpath, n.n_path)) { + Log("1 namei_ConvertROtoRWvolume: could not move LargeIndex file: %s\n", n.n_path); + code = -1; + goto done; + } OS_UNLINK(newpath); #endif diff --git a/src/volser/restorevol.c b/src/volser/restorevol.c index ea3c64f7f..e21668a8c 100644 --- a/src/volser/restorevol.c +++ b/src/volser/restorevol.c @@ -660,6 +660,7 @@ ReadVNode(afs_int32 count) int fid; int lfile; afs_sfsize_t size, s; + ssize_t count; /* Check if its vnode-file-link exists. If not, * then the file will be an orphaned file. @@ -683,14 +684,14 @@ ReadVNode(afs_int32 count) /* Write the file out */ fid = open(filename, (O_CREAT | O_WRONLY | O_TRUNC), mode); + if (fid < 0) { + fprintf(stderr, "Open %s: Errno = %d\n", filename, errno); + goto open_fail; + } size = vn.dataSize; while (size > 0) { s = (afs_int32) ((size > BUFSIZE) ? BUFSIZE : size); code = fread(buf, 1, s, dumpfile); - if (code > 0) { - (void)write(fid, buf, code); - size -= code; - } if (code != s) { if (code < 0) fprintf(stderr, "Code = %d; Errno = %d\n", code, @@ -706,6 +707,20 @@ ReadVNode(afs_int32 count) } break; } + if (code > 0) { + count = write(fid, buf, code); + if (count < 0) { + fprintf(stderr, "Count = %ld, Errno = %d\n", + (long)count, errno); + break; + } else if (count != code) { + fprintf(stderr, "Wrote %llu bytes out of %llu\n", + (afs_uintmax_t) count, + (afs_uintmax_t) code); + break; + } + size -= code; + } } close(fid); if (size != 0) { @@ -713,6 +728,7 @@ ReadVNode(afs_int32 count) filename, fname); } +open_fail: /* Remove the link to the file */ if (lfile) { unlink(filename);