From: Jeffrey Hutzelman Date: Sun, 16 Jun 2013 19:28:03 +0000 (-0400) Subject: Fix unchecked return values X-Git-Tag: upstream/1.8.0_pre1^2~407 X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=720363fa9bf7cfbebdc485104b74ca6bac1895f6;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. Change-Id: Id526f5dd7ee48be2604b77a3f00ea1e51b08c21d Reviewed-on: http://gerrit.openafs.org/9979 Tested-by: BuildBot Reviewed-by: Jeffrey Altman --- diff --git a/src/auth/cellconfig.c b/src/auth/cellconfig.c index 7f7aaaf43..2b99c0bc6 100644 --- a/src/auth/cellconfig.c +++ b/src/auth/cellconfig.c @@ -453,7 +453,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; int r; if (!(home_dir = getenv("HOME"))) { @@ -462,8 +462,6 @@ afsconf_Open(const char *adir) if (fp == 0) goto fail; - fgets(afs_confdir, 128, fp); - fclose(fp); } else { char *pathname = NULL; @@ -480,10 +478,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 07f966e79..ca85c229e 100644 --- a/src/bozo/bosserver.c +++ b/src/bozo/bosserver.c @@ -253,32 +253,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; } @@ -975,21 +989,32 @@ main(int argc, char **argv, char **envp) */ #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 */ /* create useful dirs */ - CreateDirs(DoCore); + i = CreateDirs(DoCore); + if (i) { + printf("bosserver: could not set up directories, code %d\n", i); + exit(1); + } /* Write current state of directory permissions to log file */ DirAccessOK(); /* 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); + } /* try to read the key from the config file */ tdir = afsconf_Open(AFSDIR_SERVER_ETC_DIRPATH); diff --git a/src/budb/db_text.c b/src/budb/db_text.c index 1d1a3b4a9..e88ef51e7 100644 --- a/src/budb/db_text.c +++ b/src/budb/db_text.c @@ -457,22 +457,28 @@ 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 90d9b1e5e..979e54416 100644 --- a/src/kauth/kkids.c +++ b/src/kauth/kkids.c @@ -387,8 +387,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(); @@ -429,7 +431,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 e521ff49b..50228b4c2 100644 --- a/src/lwp/lwp.c +++ b/src/lwp/lwp.c @@ -979,11 +979,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 0fcfbe19e..00b7deae6 100644 --- a/src/ptserver/pt_util.c +++ b/src/ptserver/pt_util.c @@ -394,7 +394,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); @@ -493,7 +497,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 f059d2cc7..9d2d9169e 100644 --- a/src/ptserver/ptubik.c +++ b/src/ptserver/ptubik.c @@ -31,6 +31,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)); @@ -38,9 +39,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 0893a9863..a66eb8801 100644 --- a/src/sys/rmtsysc.c +++ b/src/sys/rmtsysc.c @@ -57,7 +57,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 */ @@ -65,8 +65,6 @@ GetAfsServerAddr(char *syscall) if (fp == 0) { return 0; } - fgets(server_name, 128, fp); - fclose(fp); } else { char *pathname; @@ -83,10 +81,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/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 9334f0f6f..31f0d7717 100644 --- a/src/venus/up.c +++ b/src/venus/up.c @@ -184,8 +184,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 8209ab679..914024ef5 100644 --- a/src/viced/afsfileprocs.c +++ b/src/viced/afsfileprocs.c @@ -6554,13 +6554,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/state_analyzer.c b/src/viced/state_analyzer.c index 184a9c80f..6d8b2098a 100644 --- a/src/viced/state_analyzer.c +++ b/src/viced/state_analyzer.c @@ -330,7 +330,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/viced/viced.c b/src/viced/viced.c index 6072647d9..7cc7aaad1 100644 --- a/src/viced/viced.c +++ b/src/viced/viced.c @@ -365,18 +365,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 d64c08634..ca4208041 100644 --- a/src/vlserver/cnvldb.c +++ b/src/vlserver/cnvldb.c @@ -72,6 +72,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; @@ -101,8 +103,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); @@ -111,8 +123,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 fc0a14a4a..19aea95d6 100644 --- a/src/vol/namei_ops.c +++ b/src/vol/namei_ops.c @@ -326,7 +326,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 */ snprintf(filename, sizeof filename, "%s" OS_DIRSEP "%s", partition, @@ -338,8 +338,12 @@ namei_ViceREADME(char *partition) partition, INODEDIR); fd = OS_OPEN(filename, O_WRONLY | O_CREAT | O_TRUNC, 0444); if (fd != INVALID_FD) { - (void)OS_WRITE(fd, VICE_README, strlen(VICE_README)); + len = strlen(VICE_README); + if (OS_WRITE(fd, VICE_README, len) != len) + e = errno; OS_CLOSE(fd); + if (e) + errno = e; } return (errno); } @@ -1568,7 +1572,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; @@ -3165,7 +3172,11 @@ namei_ConvertROtoRWvolume(char *pname, VolumeId 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 @@ -3184,7 +3195,11 @@ namei_ConvertROtoRWvolume(char *pname, VolumeId 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 aef5627ef..0f2887513 100644 --- a/src/volser/restorevol.c +++ b/src/volser/restorevol.c @@ -653,6 +653,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. @@ -676,14 +677,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, @@ -698,6 +699,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) { @@ -705,6 +720,7 @@ ReadVNode(afs_int32 count) filename, fname); } +open_fail: /* Remove the link to the file */ if (lfile) { unlink(filename);