[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Freeipa-devel] [PATCHES] 206-209 Add default CFLAGS & fix hardened build



On 6.12.2013 13:31, Jan Cholasta wrote:
On 6.12.2013 12:57, Alexander Bokovoy wrote:
On Fri, 06 Dec 2013, Petr Viktorin wrote:
On 12/06/2013 11:52 AM, Jan Cholasta wrote:
On 5.12.2013 13:31, Alexander Bokovoy wrote:
On Thu, 05 Dec 2013, Petr Viktorin wrote:
On 12/05/2013 11:15 AM, Jan Cholasta wrote:
Hi,

the attached patches fix
<https://fedorahosted.org/freeipa/ticket/3896>.

Patch 207 should fix build failures some of you were having after
hardenening was enabled in the spec file.

Thanks!

In 209, would (ret != 1) make more sense than (ret == -1)? AFAIU zero
would also indicate a problem, if it somehow ended up being returned.
no, write() returns -1 if an error has happened and amount of the data
written otherwise. We specifically need to check that there was an
error, not that we wrote more or less than were supposed to write.

We are looking for EPIPE and EINTR mostly, since other errors make
little
difference for our case. In case of EINTR we need to repeat or the
worker thread didn't receive our shutdown request. In case of EPIPE we
will also get SIGPIPE and in general this means the other thread
died..

However, even if writing to the pipe failed, we still need to wait
until
thread dies with pthread_join(). I think returning -1 here is
premature.

Fixed, updated patches attached.

Also removed CFLAGS duplication, see patch 212.

Thanks you! The build works fine, ACK for 206-208, 212.

Alexander, is the C part OK? It seems a do-while loop would make sense
here.
I think do-while would be cleaner, purely from intent expression point
of view.


I actually like it the way it is, because it follows the

    ret = func()
    if (ret == -1) {
    }

pattern, which is used abundantly in our code.


... but I don't have a strong opinion about this, so whatever floats your boat.

--
Jan Cholasta
>From 749622ddeaf5f918c00b222304159c36d8ddedfe Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jcholast redhat com>
Date: Wed, 4 Dec 2013 18:37:18 +0100
Subject: [PATCH 1/5] Prefer user CFLAGS/CPPFLAGS over those provided by
 rpmbuild in the spec file.

https://fedorahosted.org/freeipa/ticket/3896
---
 freeipa.spec.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 08c82f2..fa382b0 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -320,8 +320,8 @@ This package contains tests that verify IPA functionality.
 %setup -n freeipa-%{version} -q
 
 %build
-export CFLAGS="$CFLAGS %{optflags}"
-export CPPFLAGS="$CPPFLAGS %{optflags}"
+export CFLAGS="%{optflags} $CFLAGS"
+export CPPFLAGS="%{optflags} $CPPFLAGS"
 %if 0%{?fedora} >= 19
 export SUPPORTED_PLATFORM=fedora19
 %else
-- 
1.8.4.2

>From 00f719e9b678825d6fc7f0902d616f56346c5bd7 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jcholast redhat com>
Date: Wed, 4 Dec 2013 18:39:44 +0100
Subject: [PATCH 2/5] Include LDFLAGS provided by rpmbuild in global LDFLAGS in
 the spec file.

Remove explicitly specified hardening flags from LDFLAGS in ipa-otpd.

https://fedorahosted.org/freeipa/ticket/3896
---
 daemons/ipa-otpd/Makefile.am | 2 +-
 freeipa.spec.in              | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-otpd/Makefile.am b/daemons/ipa-otpd/Makefile.am
index f0b7528..ed99c3e 100644
--- a/daemons/ipa-otpd/Makefile.am
+++ b/daemons/ipa-otpd/Makefile.am
@@ -1,5 +1,5 @@
 AM_CFLAGS := $(CFLAGS) @LDAP_CFLAGS@ @LIBVERTO_CFLAGS@
-AM_LDFLAGS := $(LDFLAGS) @LDAP_LIBS@ @LIBVERTO_LIBS@ @KRAD_LIBS@ -pie -Wl,-z,relro -Wl,-z,now
+AM_LDFLAGS := $(LDFLAGS) @LDAP_LIBS@ @LIBVERTO_LIBS@ @KRAD_LIBS@
 
 noinst_HEADERS = internal.h
 libexec_PROGRAMS = ipa-otpd
diff --git a/freeipa.spec.in b/freeipa.spec.in
index fa382b0..2a4d8fc 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -322,6 +322,7 @@ This package contains tests that verify IPA functionality.
 %build
 export CFLAGS="%{optflags} $CFLAGS"
 export CPPFLAGS="%{optflags} $CPPFLAGS"
+export LDFLAGS="%{__global_ldflags} $LDFLAGS"
 %if 0%{?fedora} >= 19
 export SUPPORTED_PLATFORM=fedora19
 %else
-- 
1.8.4.2

>From 85ad15d522274a711c87f92ed91889b781d7455e Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jcholast redhat com>
Date: Wed, 4 Dec 2013 18:42:36 +0100
Subject: [PATCH 3/5] Add stricter default CFLAGS to Makefile.

https://fedorahosted.org/freeipa/ticket/3896
---
 Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile b/Makefile
index 0664ddd..a722634 100644
--- a/Makefile
+++ b/Makefile
@@ -52,6 +52,9 @@ endif
 
 PYTHON ?= $(shell rpm -E %__python || echo /usr/bin/python)
 
+CFLAGS := -g -O2 -Werror -Wall -Wextra -Wformat-security -Wno-unused-parameter -Wno-sign-compare -Wno-missing-field-initializers $(CFLAGS)
+export CFLAGS
+
 all: bootstrap-autogen server tests
 	@for subdir in $(SUBDIRS); do \
 		(cd $$subdir && $(MAKE) $@) || exit 1; \
-- 
1.8.4.2

>From 71321ac2067e7016d02b0f1c55afdc00e73b3649 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jcholast redhat com>
Date: Wed, 4 Dec 2013 18:43:20 +0100
Subject: [PATCH 4/5] Fix compilation error in ipa-cldap.

https://fedorahosted.org/freeipa/ticket/3896
---
 daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap.c b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap.c
index fb63c9c..b310af3 100644
--- a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap.c
+++ b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap.c
@@ -82,7 +82,9 @@ static int ipa_cldap_stop(Slapi_PBlock *pb)
     }
 
     /* send stop signal to terminate worker thread */
-    write(ctx->stopfd[1], "", 1);
+    do {
+        ret = write(ctx->stopfd[1], "", 1);
+    } while (ret == -1 && errno == EINTR);
     close(ctx->stopfd[1]);
 
     ret = pthread_join(ctx->tid, &retval);
-- 
1.8.4.2

>From 412dbc6de8174ca0e155ddd436c097ea8e48a1a0 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jcholast redhat com>
Date: Fri, 6 Dec 2013 11:47:44 +0100
Subject: [PATCH 5/5] Remove CFLAGS duplication.

https://fedorahosted.org/freeipa/ticket/3896
---
 daemons/ipa-otpd/Makefile.am                           | 4 ++--
 daemons/ipa-sam/Makefile.am                            | 1 -
 daemons/ipa-slapi-plugins/ipa-cldap/Makefile.am        | 1 -
 daemons/ipa-slapi-plugins/ipa-dns/Makefile.am          | 1 -
 daemons/ipa-slapi-plugins/ipa-enrollment/Makefile.am   | 1 -
 daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am | 1 -
 daemons/ipa-slapi-plugins/ipa-lockout/Makefile.am      | 1 -
 daemons/ipa-slapi-plugins/ipa-modrdn/Makefile.am       | 1 -
 daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am    | 3 +--
 daemons/ipa-slapi-plugins/ipa-range-check/Makefile.am  | 1 -
 daemons/ipa-slapi-plugins/ipa-sidgen/Makefile.am       | 1 -
 daemons/ipa-slapi-plugins/ipa-uuid/Makefile.am         | 1 -
 daemons/ipa-slapi-plugins/ipa-version/Makefile.am      | 1 -
 daemons/ipa-slapi-plugins/ipa-winsync/Makefile.am      | 1 -
 freeipa.spec.in                                        | 1 -
 ipa-client/Makefile.am                                 | 1 -
 16 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/daemons/ipa-otpd/Makefile.am b/daemons/ipa-otpd/Makefile.am
index ed99c3e..af82a5f 100644
--- a/daemons/ipa-otpd/Makefile.am
+++ b/daemons/ipa-otpd/Makefile.am
@@ -1,5 +1,5 @@
-AM_CFLAGS := $(CFLAGS) @LDAP_CFLAGS@ @LIBVERTO_CFLAGS@
-AM_LDFLAGS := $(LDFLAGS) @LDAP_LIBS@ @LIBVERTO_LIBS@ @KRAD_LIBS@
+AM_CFLAGS := @LDAP_CFLAGS@ @LIBVERTO_CFLAGS@
+AM_LDFLAGS := @LDAP_LIBS@ @LIBVERTO_LIBS@ @KRAD_LIBS@
 
 noinst_HEADERS = internal.h
 libexec_PROGRAMS = ipa-otpd
diff --git a/daemons/ipa-sam/Makefile.am b/daemons/ipa-sam/Makefile.am
index e8e2250..d55a187 100644
--- a/daemons/ipa-sam/Makefile.am
+++ b/daemons/ipa-sam/Makefile.am
@@ -20,7 +20,6 @@ AM_CPPFLAGS =						\
 	-DLDAPIDIR=\""$(localstatedir)/run"\"		\
 	-DHAVE_LDAP					\
 	-I $(KRB5_UTIL_DIR)				\
-	$(AM_CFLAGS)					\
 	$(LDAP_CFLAGS)					\
 	$(KRB5_CFLAGS)					\
 	$(WARN_CFLAGS)					\
diff --git a/daemons/ipa-slapi-plugins/ipa-cldap/Makefile.am b/daemons/ipa-slapi-plugins/ipa-cldap/Makefile.am
index f669d6b..70b0883 100644
--- a/daemons/ipa-slapi-plugins/ipa-cldap/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-cldap/Makefile.am
@@ -12,7 +12,6 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)						\
 	$(WARN_CFLAGS)						\
 	$(NDRNBT_CFLAGS)					\
diff --git a/daemons/ipa-slapi-plugins/ipa-dns/Makefile.am b/daemons/ipa-slapi-plugins/ipa-dns/Makefile.am
index 6d09c8d..31b7485 100644
--- a/daemons/ipa-slapi-plugins/ipa-dns/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-dns/Makefile.am
@@ -12,7 +12,6 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)						\
 	$(WARN_CFLAGS)						\
 	$(NULL)
diff --git a/daemons/ipa-slapi-plugins/ipa-enrollment/Makefile.am b/daemons/ipa-slapi-plugins/ipa-enrollment/Makefile.am
index 7ba754a..3ce37ac 100644
--- a/daemons/ipa-slapi-plugins/ipa-enrollment/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-enrollment/Makefile.am
@@ -11,7 +11,6 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)					\
 	$(KRB5_CFLAGS)						\
 	$(WARN_CFLAGS)						\
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am b/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am
index df0c305..7099a98 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am
@@ -13,7 +13,6 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)						\
 	$(WARN_CFLAGS)						\
 	$(SSSIDMAP_CFLAGS)					\
diff --git a/daemons/ipa-slapi-plugins/ipa-lockout/Makefile.am b/daemons/ipa-slapi-plugins/ipa-lockout/Makefile.am
index 0c69f4d..6e4c31a 100644
--- a/daemons/ipa-slapi-plugins/ipa-lockout/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-lockout/Makefile.am
@@ -12,7 +12,6 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)					\
 	$(WARN_CFLAGS)						\
 	$(NULL)
diff --git a/daemons/ipa-slapi-plugins/ipa-modrdn/Makefile.am b/daemons/ipa-slapi-plugins/ipa-modrdn/Makefile.am
index 9fbd033..a3f8d4f 100644
--- a/daemons/ipa-slapi-plugins/ipa-modrdn/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-modrdn/Makefile.am
@@ -12,7 +12,6 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)					\
 	$(WARN_CFLAGS)						\
 	$(NULL)
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am b/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am
index b53b2e1..8bd8965 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am
@@ -18,13 +18,12 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)					\
 	$(KRB5_CFLAGS)						\
 	$(SSL_CFLAGS)						\
 	$(WARN_CFLAGS)						\
 	$(NULL)
-	
+
 AM_LDFLAGS = \
 	$(KRB5_LIBS)	\
 	$(SSL_LIBS)	\
diff --git a/daemons/ipa-slapi-plugins/ipa-range-check/Makefile.am b/daemons/ipa-slapi-plugins/ipa-range-check/Makefile.am
index f23a24e..5aa9b54 100644
--- a/daemons/ipa-slapi-plugins/ipa-range-check/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-range-check/Makefile.am
@@ -12,7 +12,6 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)					\
 	$(WARN_CFLAGS)						\
 	$(NULL)
diff --git a/daemons/ipa-slapi-plugins/ipa-sidgen/Makefile.am b/daemons/ipa-slapi-plugins/ipa-sidgen/Makefile.am
index 4bfb018..642fdd5 100644
--- a/daemons/ipa-slapi-plugins/ipa-sidgen/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-sidgen/Makefile.am
@@ -12,7 +12,6 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)					\
 	$(WARN_CFLAGS)						\
 	$(NULL)
diff --git a/daemons/ipa-slapi-plugins/ipa-uuid/Makefile.am b/daemons/ipa-slapi-plugins/ipa-uuid/Makefile.am
index 7382901..061d848 100644
--- a/daemons/ipa-slapi-plugins/ipa-uuid/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-uuid/Makefile.am
@@ -12,7 +12,6 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)					\
 	$(WARN_CFLAGS)						\
 	$(NULL)
diff --git a/daemons/ipa-slapi-plugins/ipa-version/Makefile.am b/daemons/ipa-slapi-plugins/ipa-version/Makefile.am
index 5396bda..afce915 100644
--- a/daemons/ipa-slapi-plugins/ipa-version/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-version/Makefile.am
@@ -13,7 +13,6 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)					\
 	$(KRB5_CFLAGS)						\
 	$(WARN_CFLAGS)						\
diff --git a/daemons/ipa-slapi-plugins/ipa-winsync/Makefile.am b/daemons/ipa-slapi-plugins/ipa-winsync/Makefile.am
index c416928..3108f3c 100644
--- a/daemons/ipa-slapi-plugins/ipa-winsync/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-winsync/Makefile.am
@@ -11,7 +11,6 @@ AM_CPPFLAGS =							\
 	-DLIBDIR=\""$(libdir)"\" 				\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
-	$(AM_CFLAGS)						\
 	$(LDAP_CFLAGS)					\
 	$(WARN_CFLAGS)						\
 	$(NULL)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 2a4d8fc..b7c4b72 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -321,7 +321,6 @@ This package contains tests that verify IPA functionality.
 
 %build
 export CFLAGS="%{optflags} $CFLAGS"
-export CPPFLAGS="%{optflags} $CPPFLAGS"
 export LDFLAGS="%{__global_ldflags} $LDFLAGS"
 %if 0%{?fedora} >= 19
 export SUPPORTED_PLATFORM=fedora19
diff --git a/ipa-client/Makefile.am b/ipa-client/Makefile.am
index ca251ae..2df175e 100644
--- a/ipa-client/Makefile.am
+++ b/ipa-client/Makefile.am
@@ -25,7 +25,6 @@ AM_CPPFLAGS =							\
 	-DLIBEXECDIR=\""$(libexecdir)"\"			\
 	-DDATADIR=\""$(datadir)"\"				\
 	-DLOCALEDIR=\""$(localedir)"\"				\
-	$(AM_CFLAGS)						\
 	$(KRB5_CFLAGS)						\
 	$(OPENLDAP_CFLAGS)					\
 	$(SASL_CFLAGS)						\
-- 
1.8.4.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]