Created attachment 9060 [details] Adds Sanity Check for ftell() call Hello All, In reviewing various files in Samba-4.0.7, I found a number of instances where ftell() and/or fcntl() were called without the checking the return value for a value of -1, which would indicate failure. In directory 'samba-4.0.7/source3/passdb', file 'pdb_smbpasswd.c', the following patch file makes the required check, though in some cases, a(n) error message to stderr might be appropriate: --- pdb_smbpasswd.c.orig 2013-07-17 19:04:25.962920605 -0700 +++ pdb_smbpasswd.c 2013-07-17 19:07:37.747919091 -0700 @@ -790,6 +790,11 @@ status = linebuf; while (status && !feof(fp)) { pwd_seekpos = ftell(fp); + if (pwd_seekpos == -1) { + DEBUG(0, ("mod_smbfilepwd_entry: unable to reposition file%\n", pfile)); + fclose(fp); + return False; + } linebuf[0] = '\0'; In reviewing various files in Samba-4.0.7, I found a number of instances where fcntl() were called without the checking the return value for a value of -1, which would indicate failure. In directory 'samba-4.0.7/lib/async_req', file 'async_sock.c' the following patch file makes the required check, though in some cases, a(n) error message to stderr might be appropriate: --- async_sock.c.orig 2013-07-17 19:22:43.493922674 -0700 +++ async_sock.c 2013-07-17 19:29:06.393912269 -0700 @@ -306,7 +306,10 @@ post_errno: tevent_req_error(result, state->sys_errno); done: - fcntl(fd, F_SETFL, state->old_sockflags); + if (fcntl(fd, F_SETFL, state->old_sockflags) == -1) { + state->sys_errno = errno; + tevent_req_error(result, state->sys_errno); + } return tevent_req_post(result, ev); } @@ -352,7 +355,9 @@ tevent_req_data(req, struct async_connect_state); int err; - fcntl(state->fd, F_SETFL, state->old_sockflags); + if (fcntl(state->fd, F_SETFL, state->old_sockflags) == -1) { + return -1; + } if (tevent_req_is_unix_error(req, &err)) { *perrno = err; In directory 'samba-4.0.7/source3/lib', file 'recvfile.c' the following patch file makes the required check, though in some cases, a(n) error message to stderr might be appropriate: --- recvfile.c.orig 2013-07-17 19:50:43.587919082 -0700 +++ recvfile.c 2013-07-17 19:51:51.016918916 -0700 @@ -260,6 +260,11 @@ } old_flags = fcntl(sockfd, F_GETFL, 0); + if (old_flags == -1) { + free(buffer); + return -1; + } + if (set_blocking(sockfd, true) == -1) { free(buffer); return -1; In directory 'samba-4.0.7/source3/smbd', file 'vfs.c' the following patch file makes the required check, though in some cases, a(n) error message to stderr might be appropriate: --- vfs.c.orig 2013-07-17 19:56:28.243919876 -0700 +++ vfs.c 2013-07-17 19:57:58.805916306 -0700 @@ -436,6 +436,9 @@ req->unread_bytes = 0; /* Ensure the socket is blocking. */ old_flags = fcntl(sockfd, F_GETFL, 0); + if (old_flags == -1) { + return (ssize_t)-1; + } if (set_blocking(sockfd, true) == -1) { return (ssize_t)-1; } @@ -480,6 +483,9 @@ req->unread_bytes = 0; /* Ensure the socket is blocking. */ old_flags = fcntl(sockfd, F_GETFL, 0); + if (old_flags == -1) { + return (ssize_t)-1; + } if (set_blocking(sockfd, true) == -1) { return (ssize_t)-1; } In directory 'samba-4.0.7/tests', file 'fcntl_lock_thread.c' the following patch file makes the required check, though in some cases, a(n) error message to stderr might be appropriate: --- fcntl_lock_thread.c.orig 2013-07-17 20:04:39.181918422 -0700 +++ fcntl_lock_thread.c 2013-07-17 20:07:17.767919133 -0700 @@ -97,7 +97,10 @@ lock.l_pid = getpid(); /* set a 4 byte write lock */ - fcntl(fd,F_SETLK,&lock); + ret = fcntl(fd,F_SETLK,&lock); + if (ret == -1) { + fprintf(stderr,"ERROR: lock test failed (ret=%d errno=%d)\n", ret, (int)errno); + } sleep(4); /* allow thread to try getting lock */ I am attaching these patches to the bug report Bill Parker (wp02855 at gmail dot com)
Created attachment 9061 [details] Adds Sanity Check for fcntl() call
Created attachment 9062 [details] Adds Sanity Check for fcntl() call
Created attachment 9063 [details] Adds Sanity Check for fcntl() call
Created attachment 9064 [details] Adds Sanity Check for fcntl() call