2014-07-06 00:38:49

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH] fix cross-compilation support

Commit 2500ebfc89 (util: fix make dependencies for subst) broke cross
compilation because it unconditionally used config.h without setting a
includes path so that the config.h file could be found.

The proposed fix of adding the include path (such as was proposed at
http://patchwork.ozlabs.org/patch/355662/ or in Debian Bug #753375)
aren't really the right fix, since the information in config.h is for
the target environment, and not the build environment. So using
config.h when building helper programs used as part of the build can
potentially cause more problems than it solves.

In general, build helpers must be written to be as portable as
possible, and to not require any autoconf defined #ifdef's whenever
possible. The subst program broke this rule to (1) address a Coverity
security complaint by using futimes if is present, and (2) to preserve
the nanosecond portion of the file timestamp.

Oh, well. We won't be able to do the latter when cross compiling, and
as to the former, if an attacker has write access to your build tree
while you are building programs that will be run as root, you've got
bigger problems. :-)

Fix the problem that commit 2500ebfc89 was trying to address by
explicitly adding @DEFS@ to CFLAGS, so that -DHAVE_CONFIG_H is passed
to make depend. This fixes up the make depend without forcing the use
of config.h when cross-compiling.

Addresses-Debian-Bug: #753375
Signed-off-by: Theodore Ts'o <[email protected]>
Cc: Helmut Grohne <[email protected]>
Cc: Gustavo Zacarias <[email protected]>
Cc: [email protected]
---
MCONFIG.in | 2 +-
util/subst.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/MCONFIG.in b/MCONFIG.in
index 3c1a4cb..7393379 100644
--- a/MCONFIG.in
+++ b/MCONFIG.in
@@ -67,7 +67,7 @@ pkgconfigdir = $(libdir)/pkgconfig

CC = @CC@
BUILD_CC = @BUILD_CC@
-CFLAGS = @CFLAGS@
+CFLAGS = @CFLAGS@ @DEFS@
CPPFLAGS = @INCLUDES@
ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
LDFLAGS = @LDFLAGS@
diff --git a/util/subst.c b/util/subst.c
index 6a5eab1..6244831 100644
--- a/util/subst.c
+++ b/util/subst.c
@@ -5,7 +5,9 @@
*
*/

+#ifdef HAVE_CONFIG_H
#include "config.h"
+#endif
#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
--
2.0.0



2014-07-06 00:45:26

by Gustavo Zacarias

[permalink] [raw]
Subject: Re: [PATCH] fix cross-compilation support

On 07/05/2014 09:38 PM, Theodore Ts'o wrote:

> Commit 2500ebfc89 (util: fix make dependencies for subst) broke cross
> compilation because it unconditionally used config.h without setting a
> includes path so that the config.h file could be found.
>
> The proposed fix of adding the include path (such as was proposed at
> http://patchwork.ozlabs.org/patch/355662/ or in Debian Bug #753375)
> aren't really the right fix, since the information in config.h is for
> the target environment, and not the build environment. So using
> config.h when building helper programs used as part of the build can
> potentially cause more problems than it solves.
>
> In general, build helpers must be written to be as portable as
> possible, and to not require any autoconf defined #ifdef's whenever
> possible. The subst program broke this rule to (1) address a Coverity
> security complaint by using futimes if is present, and (2) to preserve
> the nanosecond portion of the file timestamp.
>
> Oh, well. We won't be able to do the latter when cross compiling, and
> as to the former, if an attacker has write access to your build tree
> while you are building programs that will be run as root, you've got
> bigger problems. :-)
>
> Fix the problem that commit 2500ebfc89 was trying to address by
> explicitly adding @DEFS@ to CFLAGS, so that -DHAVE_CONFIG_H is passed
> to make depend. This fixes up the make depend without forcing the use
> of config.h when cross-compiling.
>
> Addresses-Debian-Bug: #753375
> Signed-off-by: Theodore Ts'o <[email protected]>
> Cc: Helmut Grohne <[email protected]>
> Cc: Gustavo Zacarias <[email protected]>
> Cc: [email protected]

Tested-by: Gustavo Zacarias <[email protected]>

Indeed cleaner than my solution, thanks.
Regards.