Attachment 2823 Details for Bug 2581 – Proposed patch for points 6 - 10

[patch] Proposed patch for points 6 - 10

coverity.ssh2.patch (text/plain), 8.67 KB, created by Jakub Jelen on 2016-06-03 22:33:05 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Jakub Jelen

Created: 2016-06-03 22:33:05 AEST

Size: 8.67 KB

patch

obsolete

>commit 92243e06c41257d0391eb6b6093a78846973f355
>Author: Jakub Jelen <jjelen@redhat.com>
>Date:   Fri Jun 3 14:31:42 2016 +0200
>
>    Coverity fd checks, const paths, return values
>
>diff --git a/auth-pam.c b/auth-pam.c
>index f80278c..074801c 100644
>--- a/auth-pam.c
>+++ b/auth-pam.c
>@@ -217,7 +217,12 @@ pthread_join(sp_pthread_t thread, void **value)
> 	if (sshpam_thread_status != -1)
> 		return (sshpam_thread_status);
> 	signal(SIGCHLD, sshpam_oldsig);
>-	waitpid(thread, &status, 0);
>+	while (waitpid(thread, &status, 0) < 0) {
>+		if (errno == EINTR)
>+			continue;
>+		fatal("%s: waitpid: %s", __func__,
>+				strerror(errno));
>+	}
> 	return (status);
> }
> #endif
>diff --git a/channels.c b/channels.c
>index 2601ad5..d39123f 100644
>--- a/channels.c
>+++ b/channels.c
>@@ -246,11 +246,11 @@ channel_register_fds(Channel *c, int rfd, int wfd, int efd,
> 	channel_max_fd = MAX(channel_max_fd, wfd);
> 	channel_max_fd = MAX(channel_max_fd, efd);
> 
>-	if (rfd != -1)
>+	if (rfd >= 0)
> 		fcntl(rfd, F_SETFD, FD_CLOEXEC);
>-	if (wfd != -1 && wfd != rfd)
>+	if (wfd >= 0 && wfd != rfd)
> 		fcntl(wfd, F_SETFD, FD_CLOEXEC);
>-	if (efd != -1 && efd != rfd && efd != wfd)
>+	if (efd >= 0 && efd != rfd && efd != wfd)
> 		fcntl(efd, F_SETFD, FD_CLOEXEC);
> 
> 	c->rfd = rfd;
>@@ -268,11 +268,11 @@ channel_register_fds(Channel *c, int rfd, int wfd, int efd,
> 
> 	/* enable nonblocking mode */
> 	if (nonblock) {
>-		if (rfd != -1)
>+		if (rfd >= 0)
> 			set_nonblock(rfd);
>-		if (wfd != -1)
>+		if (wfd >= 0)
> 			set_nonblock(wfd);
>-		if (efd != -1)
>+		if (efd >= 0)
> 			set_nonblock(efd);
> 	}
> }
>diff --git a/monitor.c b/monitor.c
>index 9a52f50..06ff460 100644
>--- a/monitor.c
>+++ b/monitor.c
>@@ -420,7 +420,7 @@ monitor_child_preauth(Authctxt *_authctxt, struct monitor *pmonitor)
> 	mm_get_keystate(pmonitor);
> 
> 	/* Drain any buffered messages from the child */
>-	while (pmonitor->m_log_recvfd != -1 && monitor_read_log(pmonitor) == 0)
>+	while (pmonitor->m_log_recvfd >= 0 && monitor_read_log(pmonitor) == 0)
> 		;
> 
> 	close(pmonitor->m_sendfd);
>diff --git a/scp.c b/scp.c
>index 3f0d750..15eef0d 100644
>--- a/scp.c
>+++ b/scp.c
>@@ -156,7 +156,7 @@ killchild(int signo)
> {
> 	if (do_cmd_pid > 1) {
> 		kill(do_cmd_pid, signo ? signo : SIGTERM);
>-		waitpid(do_cmd_pid, NULL, 0);
>+		(void) waitpid(do_cmd_pid, NULL, 0);
> 	}
> 
> 	if (signo)
>diff --git a/servconf.c b/servconf.c
>index d5f77df..5515658 100644
>--- a/servconf.c
>+++ b/servconf.c
>@@ -1502,7 +1502,7 @@ process_server_config_line(ServerOptions *options, char *line,
> 			fatal("%s line %d: Missing subsystem name.",
> 			    filename, linenum);
> 		if (!*activep) {
>-			arg = strdelim(&cp);
>+			(void) strdelim(&cp);
> 			break;
> 		}
> 		for (i = 0; i < options->num_subsystems; i++)
>diff --git a/serverloop.c b/serverloop.c
>index 3563e5d..89bf5dc 100644
>--- a/serverloop.c
>+++ b/serverloop.c
>@@ -146,13 +146,13 @@ notify_setup(void)
> static void
> notify_parent(void)
> {
>-	if (notify_pipe[1] != -1)
>+	if (notify_pipe[1] >= 0)
> 		(void)write(notify_pipe[1], "", 1);
> }
> static void
> notify_prepare(fd_set *readset)
> {
>-	if (notify_pipe[0] != -1)
>+	if (notify_pipe[0] >= 0)
> 		FD_SET(notify_pipe[0], readset);
> }
> static void
>@@ -160,7 +160,7 @@ notify_done(fd_set *readset)
> {
> 	char c;
> 
>-	if (notify_pipe[0] != -1 && FD_ISSET(notify_pipe[0], readset))
>+	if (notify_pipe[0] >= 0 && FD_ISSET(notify_pipe[0], readset))
> 		while (read(notify_pipe[0], &c, 1) != -1)
> 			debug2("notify_done: reading");
> }
>@@ -338,7 +338,7 @@ wait_until_can_do_something(fd_set **readsetp, fd_set **writesetp, int *maxfdp,
> 		 * If we have buffered data, try to write some of that data
> 		 * to the program.
> 		 */
>-		if (fdin != -1 && buffer_len(&stdin_buffer) > 0)
>+		if (fdin >= 0 && buffer_len(&stdin_buffer) > 0)
> 			FD_SET(fdin, *writesetp);
> 	}
> 	notify_prepare(*readsetp);
>@@ -477,7 +477,7 @@ process_output(fd_set *writeset)
> 	int len;
> 
> 	/* Write buffered data to program stdin. */
>-	if (!compat20 && fdin != -1 && FD_ISSET(fdin, writeset)) {
>+	if (!compat20 && fdin >= 0 && FD_ISSET(fdin, writeset)) {
> 		data = buffer_ptr(&stdin_buffer);
> 		dlen = buffer_len(&stdin_buffer);
> 		len = write(fdin, data, dlen);
>@@ -590,7 +590,7 @@ server_loop(pid_t pid, int fdin_arg, int fdout_arg, int fderr_arg)
> 	set_nonblock(fdin);
> 	set_nonblock(fdout);
> 	/* we don't have stderr for interactive terminal sessions, see below */
>-	if (fderr != -1)
>+	if (fderr >= 0)
> 		set_nonblock(fderr);
> 
> 	if (!(datafellows & SSH_BUG_IGNOREMSG) && isatty(fdin))
>@@ -644,7 +644,7 @@ server_loop(pid_t pid, int fdin_arg, int fdout_arg, int fderr_arg)
> 		 * If we have received eof, and there is no more pending
> 		 * input data, cause a real eof by closing fdin.
> 		 */
>-		if (stdin_eof && fdin != -1 && buffer_len(&stdin_buffer) == 0) {
>+		if (stdin_eof && fdin >= 0 && buffer_len(&stdin_buffer) == 0) {
> 			if (fdin != fdout)
> 				close(fdin);
> 			else
>@@ -740,15 +740,15 @@ server_loop(pid_t pid, int fdin_arg, int fdout_arg, int fderr_arg)
> 	buffer_free(&stderr_buffer);
> 
> 	/* Close the file descriptors. */
>-	if (fdout != -1)
>+	if (fdout >= 0)
> 		close(fdout);
> 	fdout = -1;
> 	fdout_eof = 1;
>-	if (fderr != -1)
>+	if (fderr >= 0)
> 		close(fderr);
> 	fderr = -1;
> 	fderr_eof = 1;
>-	if (fdin != -1)
>+	if (fdin >= 0)
> 		close(fdin);
> 	fdin = -1;
> 
>@@ -944,7 +944,7 @@ server_input_window_size(int type, u_int32_t seq, void *ctxt)
> 
> 	debug("Window change received.");
> 	packet_check_eom();
>-	if (fdin != -1)
>+	if (fdin >= 0)
> 		pty_change_window_size(fdin, row, col, xpixel, ypixel);
> 	return 0;
> }
>diff --git a/sftp.c b/sftp.c
>index 3d2d13a..d6c35f4 100644
>--- a/sftp.c
>+++ b/sftp.c
>@@ -223,7 +223,7 @@ killchild(int signo)
> {
> 	if (sshpid > 1) {
> 		kill(sshpid, SIGTERM);
>-		waitpid(sshpid, NULL, 0);
>+		(void) waitpid(sshpid, NULL, 0);
> 	}
> 
> 	_exit(1);
>@@ -335,7 +335,7 @@ local_do_ls(const char *args)
> 
> /* Strip one path (usually the pwd) from the start of another */
> static char *
>-path_strip(char *path, char *strip)
>+path_strip(const char *path, const char *strip)
> {
> 	size_t len;
> 
>@@ -353,7 +353,7 @@ path_strip(char *path, char *strip)
> }
> 
> static char *
>-make_absolute(char *p, char *pwd)
>+make_absolute(char *p, const char *pwd)
> {
> 	char *abs_str;
> 
>@@ -551,7 +551,7 @@ parse_no_flags(const char *cmd, char **argv, int argc)
> }
> 
> static int
>-is_dir(char *path)
>+is_dir(const char *path)
> {
> 	struct stat sb;
> 
>@@ -563,7 +563,7 @@ is_dir(char *path)
> }
> 
> static int
>-remote_is_dir(struct sftp_conn *conn, char *path)
>+remote_is_dir(struct sftp_conn *conn, const char *path)
> {
> 	Attrib *a;
> 
>@@ -577,7 +577,7 @@ remote_is_dir(struct sftp_conn *conn, char *path)
> 
> /* Check whether path returned from glob(..., GLOB_MARK, ...) is a directory */
> static int
>-pathname_is_dir(char *pathname)
>+pathname_is_dir(const char *pathname)
> {
> 	size_t l = strlen(pathname);
> 
>@@ -585,8 +585,8 @@ pathname_is_dir(char *pathname)
> }
> 
> static int
>-process_get(struct sftp_conn *conn, char *src, char *dst, char *pwd,
>-    int pflag, int rflag, int resume, int fflag)
>+process_get(struct sftp_conn *conn, const char *src, const char *dst,
>+    const char *pwd, int pflag, int rflag, int resume, int fflag)
> {
> 	char *abs_src = NULL;
> 	char *abs_dst = NULL;
>@@ -669,8 +669,8 @@ out:
> }
> 
> static int
>-process_put(struct sftp_conn *conn, char *src, char *dst, char *pwd,
>-    int pflag, int rflag, int resume, int fflag)
>+process_put(struct sftp_conn *conn, const char *src, const char *dst,
>+    const char *pwd, int pflag, int rflag, int resume, int fflag)
> {
> 	char *tmp_dst = NULL;
> 	char *abs_dst = NULL;
>@@ -779,7 +779,7 @@ sdirent_comp(const void *aa, const void *bb)
> 
> /* sftp ls.1 replacement for directories */
> static int
>-do_ls_dir(struct sftp_conn *conn, char *path, char *strip_path, int lflag)
>+do_ls_dir(struct sftp_conn *conn, const char *path, const char *strip_path, int lflag)
> {
> 	int n;
> 	u_int c = 1, colspace = 0, columns = 1;
>@@ -864,7 +864,7 @@ do_ls_dir(struct sftp_conn *conn, char *path, char *strip_path, int lflag)
> 
> /* sftp ls.1 replacement which handles path globs */
> static int
>-do_globbed_ls(struct sftp_conn *conn, char *path, char *strip_path,
>+do_globbed_ls(struct sftp_conn *conn, const char *path, const char *strip_path,
>     int lflag)
> {
> 	char *fname, *lname;
>@@ -949,7 +949,7 @@ do_globbed_ls(struct sftp_conn *conn, char *path, char *strip_path,
> }
> 
> static int
>-do_df(struct sftp_conn *conn, char *path, int hflag, int iflag)
>+do_df(struct sftp_conn *conn, const char *path, int hflag, int iflag)
> {
> 	struct sftp_statvfs st;
> 	char s_used[FMT_SCALED_STRSIZE];
>diff --git a/ssh-agent.c b/ssh-agent.c
>index 8aa25b3..0679847 100644
>--- a/ssh-agent.c
>+++ b/ssh-agent.c
>@@ -1206,8 +1206,8 @@ main(int ac, char **av)
> 	sanitise_stdfd();
> 
> 	/* drop */
>-	setegid(getgid());
>-	setgid(getgid());
>+	(void) setegid(getgid());
>+	(void) setgid(getgid());
> 
> #if defined(HAVE_PRCTL) && defined(PR_SET_DUMPABLE)
> 	/* Disable ptrace on Linux without sgid bit */

Actions: View | Diff

Attachments on bug 2581: 2822 | 2823