additional compiler hardening flags

Corinna Vinschen vinschen at redhat.com
Sat Jan 18 03:03:54 EST 2014


On Jan 17 15:59, Corinna Vinschen wrote:
> On Jan 17 10:10, Darren Tucker wrote:
> > On Thu, Jan 16, 2014 at 11:44:26PM +0100, Corinna Vinschen wrote:
> > > Will you accept patches to get rid of the existing warnings so we can
> > > build all of OpenSSH with -Werror?
> > [...]
> I don't think I have any warning which is too intrusive to fix.
> Let me see, I've just started building the latest from CVS.
> 
> [...time passes...]
> 
> Here's the first one:
>

[...more time passes...]

And here's the rest of them:


"signed - unsigned comparisons".  The reason is that socklen_t
is signed, but sizeof returns a size_t value (addrmatch.c), resp
i was defined as unsigend type (canohost.c).

Index: addrmatch.c
===================================================================
RCS file: /cvs/openssh/addrmatch.c,v
retrieving revision 1.7
diff -u -p -r1.7 addrmatch.c
--- addrmatch.c	1 Jun 2013 21:31:18 -0000	1.7
+++ addrmatch.c	17 Jan 2014 15:52:17 -0000
@@ -88,13 +88,13 @@ addr_sa_to_xaddr(struct sockaddr *sa, so
 
 	switch (sa->sa_family) {
 	case AF_INET:
-		if (slen < sizeof(*in4))
+		if ((size_t) slen < sizeof(*in4))
 			return -1;
 		xa->af = AF_INET;
 		memcpy(&xa->v4, &in4->sin_addr, sizeof(xa->v4));
 		break;
 	case AF_INET6:
-		if (slen < sizeof(*in6))
+		if ((size_t) slen < sizeof(*in6))
 			return -1;
 		xa->af = AF_INET6;
 		memcpy(&xa->v6, &in6->sin6_addr, sizeof(xa->v6));
Index: canohost.c
===================================================================
RCS file: /cvs/openssh/canohost.c,v
retrieving revision 1.80
diff -u -p -r1.80 canohost.c
--- canohost.c	21 Nov 2013 02:57:15 -0000	1.80
+++ canohost.c	17 Jan 2014 15:52:17 -0000
@@ -155,7 +155,7 @@ check_ip_options(int sock, char *ipaddr)
 	u_char options[200];
 	char text[sizeof(options) * 3 + 1];
 	socklen_t option_size;
-	u_int i;
+	socklen_t i;
 	int ipproto;
 	struct protoent *ip;
 

A cygwin problem: The getopt variables (like optargs, optind) are
defined in getopt.h already.  Unfortunately they are defined as
"declspec(dllimport)" for historical reasons, because the GNU linker
didn't allow auto-import on PE/COFF targets way back when.  The problem
is, the dllexport attributes collide with the definitions in the various
source files in OpenSSH, which obviousy define the variables without 
declspec(dllimport).  The least intrusive way to get rid of these
warnings is to disable warnings for GCC compiler attributes.

Index: configure.ac
===================================================================
RCS file: /cvs/openssh/configure.ac,v
retrieving revision 1.553
diff -u -p -r1.553 configure.ac
--- configure.ac	17 Jan 2014 08:17:35 -0000	1.553
+++ configure.ac	17 Jan 2014 15:52:18 -0000
@@ -155,6 +155,7 @@ if test "$GCC" = "yes" || test "$GCC" = 
 	OSSH_CHECK_CFLAG_COMPILE([-Qunused-arguments])
 	OSSH_CHECK_CFLAG_COMPILE([-Wunknown-warning-option])
 	OSSH_CHECK_CFLAG_COMPILE([-Wall])
+	OSSH_CHECK_CFLAG_COMPILE([-Wno-attributes])
 	OSSH_CHECK_CFLAG_COMPILE([-Wpointer-arith])
 	OSSH_CHECK_CFLAG_COMPILE([-Wuninitialized])
 	OSSH_CHECK_CFLAG_COMPILE([-Wsign-compare])


The definition of USE_PIPES in session.c collides with the
definition in config.h:

Index: session.c
===================================================================
RCS file: /cvs/openssh/session.c,v
retrieving revision 1.424
diff -u -p -r1.424 session.c
--- session.c	30 Oct 2013 11:21:50 -0000	1.424
+++ session.c	17 Jan 2014 15:52:18 -0000
@@ -441,7 +441,7 @@ do_authenticated1(Authctxt *authctxt)
 	}
 }
 
-#define USE_PIPES
+#define USE_PIPES 1
 /*
  * This is called to fork and execute a command when we have no tty.  This
  * will call do_child from the child, and server_loop from the parent after


GCC is unhappy: error: no return statement in function returning non-void
[-Werror=return-type]:

Index: ssh-keyscan.c
===================================================================
RCS file: /cvs/openssh/ssh-keyscan.c,v
retrieving revision 1.110
diff -u -p -r1.110 ssh-keyscan.c
--- ssh-keyscan.c	7 Dec 2013 00:24:02 -0000	1.110
+++ ssh-keyscan.c	17 Jan 2014 15:52:18 -0000
@@ -221,6 +221,7 @@ hostjump(Key *hostkey)
 {
 	kexjmp_key = hostkey;
 	longjmp(kexjmp, 1);
+	return 0;		/* Silence compiler warnings. */
 }
 
 static int


Cygwin-only:   error: unused variable ‘old_uid’ [-Werror=unused-variable]
               error: unused variable ‘old_gid’ [-Werror=unused-variable]

Index: uidswap.c
===================================================================
RCS file: /cvs/openssh/uidswap.c,v
retrieving revision 1.63
diff -u -p -r1.63 uidswap.c
--- uidswap.c	21 Nov 2013 02:55:44 -0000	1.63
+++ uidswap.c	17 Jan 2014 15:52:18 -0000
@@ -134,7 +134,9 @@ temporarily_use_uid(struct passwd *pw)
 void
 permanently_drop_suid(uid_t uid)
 {
+#ifndef HAVE_CYGWIN
 	uid_t old_uid = getuid();
+#endif
 
 	debug("permanently_drop_suid: %u", (u_int)uid);
 	if (setresuid(uid, uid, uid) < 0)
@@ -197,8 +199,10 @@ restore_uid(void)
 void
 permanently_set_uid(struct passwd *pw)
 {
+#ifndef HAVE_CYGWIN
 	uid_t old_uid = getuid();
 	gid_t old_gid = getgid();
+#endif
 
 	if (pw == NULL)
 		fatal("permanently_set_uid: no user given");

Cygwin-only: Missing function declarations because a couple of months
ago we removed `#include <windows.h> from openbsd-compat/bsd-cygwin_util.h.
The problem:  The function declarations are only enabled in the system
header, if windows.h is included.  Duh.

Index: openbsd-compat/bsd-cygwin_util.h
===================================================================
RCS file: /cvs/openssh/openbsd-compat/bsd-cygwin_util.h,v
retrieving revision 1.16
diff -u -p -r1.16 bsd-cygwin_util.h
--- openbsd-compat/bsd-cygwin_util.h	1 Apr 2013 01:40:49 -0000	1.16
+++ openbsd-compat/bsd-cygwin_util.h	17 Jan 2014 15:52:19 -0000
@@ -40,8 +40,14 @@
 typedef void *HANDLE;
 #define INVALID_HANDLE_VALUE ((HANDLE) -1)
 
+/* Cygwin functions for which declarations are only available when including
+   windows headers, so we have to define them here explicitely. */
+extern HANDLE cygwin_logon_user (const struct passwd *, const char *);
+extern void cygwin_set_impersonation_token (const HANDLE);
+
 #include <sys/cygwin.h>
 #include <io.h>
+
 
 int binary_open(const char *, int , ...);
 int check_ntsec(const char *);


That's it, as far as the warnings are concerned.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20140117/5dffcbf4/attachment.bin>


More information about the openssh-unix-dev mailing list