[PATCH 0/2] Small multipath fixes

[PATCH 0/2] Small multipath fixes

[Martin Wilck]

Two minor fixes that I found necessary recently.

Both touch potentially bigger issues - the first one print.c, which
might need some general overhaul at some time in the future, and the
second one the build process of multipath tools, which is currently
lacking an "autobuild" mechanism.

My intention at was to keep the patches small and non-controversial,
therefore I didn't delve into the big issues just yet.

Martin Wilck (2):
  libmultipath: print.c: make sure lines are 0-terminated
  multipath-tools: fix compilation with gcc < 4.9

 Makefile.inc         | 15 ++++++++++++++-
 libmultipath/print.c | 10 ++++++++--
 2 files changed, 22 insertions(+), 3 deletions(-)

-- 
2.12.2

[PATCH 1/2] libmultipath: print.c: make sure lines are 0-terminated

[Martin Wilck]

Under certain conditions, the output of "multipath -ll" or
"multipathd show topology", or the multipathd logs, may
contain garbage characters. Fix that by making sure that
the strings are always properly zero-terminated.

Note 1: The way this is coded, the previously written
character is overwritten by the newline. That behavior is
unchanged by this patch. I didn't want to fuzz with the
carefully crafted print.c code more than necessary at
this point.

Note 2: The condition (c <= line + len - 1) is equivalent to
(TAIL >= 0). It should always hold the way ENDLINE is called
after PRINT and PAD in print.c, and is only added as an
additional safeguard, e.g. against future code changes.
---
 libmultipath/print.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 7c2a1588..3a07fcf5 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -40,8 +40,14 @@ do { \
 } while (0)
 
 #define ENDLINE \
-		if (c > line) \
-			line[c - line - 1] = '\n'
+		if (c > line) {				\
+			if (c <= line + len - 1) {	\
+				*(c - 1) = '\n';	\
+				*c = '\0';		\
+			} else				\
+				line[len - 1] = '\0';	\
+		}
+
 #define PRINT(var, size, format, args...) \
 do { \
 	fwd = snprintf(var, size, format, ##args); \
-- 
2.12.2

Re: [PATCH 1/2] libmultipath: print.c: make sure lines are 0-terminated

[Johannes Thumshirn]

Hi Martin,

On 05/17/2017 03:54 PM, Martin Wilck wrote:
> Under certain conditions, the output of "multipath -ll" or
> "multipathd show topology", or the multipathd logs, may
> contain garbage characters. Fix that by making sure that
> the strings are always properly zero-terminated.
> 
> Note 1: The way this is coded, the previously written
> character is overwritten by the newline. That behavior is
> unchanged by this patch. I didn't want to fuzz with the
> carefully crafted print.c code more than necessary at
> this point.
> 
> Note 2: The condition (c <= line + len - 1) is equivalent to
> (TAIL >= 0). It should always hold the way ENDLINE is called
> after PRINT and PAD in print.c, and is only added as an
> additional safeguard, e.g. against future code changes.

^^ Missing S-o-b


-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [PATCH 1/2] libmultipath: print.c: make sure lines are 0-terminated

[Bart Van Assche]

On Wed, 2017-05-17 at 15:54 +0200, Martin Wilck wrote:
>  #define ENDLINE \
> -		if (c > line) \
> -			line[c - line - 1] = '\n'
> +		if (c > line) {				\
> +			if (c <= line + len - 1) {	\
> +				*(c - 1) = '\n';	\
> +				*c = '\0';		\
> +			} else				\
> +				line[len - 1] = '\0';	\
> +		}

Hello Martin,

Please convert ENDLINE from a macro into a function. Functions are
easier to maintain than macros.

Thanks,

Bart.

[PATCH 2/2] multipath-tools: fix compilation with gcc < 4.9

[Martin Wilck]

gcc supports -fstack-protector-strong since v4.9 only. Add a "poor man's
autoconf" test to check whether the option is supported on the given
build system.

Fixes: 596f51f3
"multipath-tools: Replace -fstack-protector with -fstack-protector-strong"

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 Makefile.inc | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/Makefile.inc b/Makefile.inc
index 8361e6c4..3ef134f6 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -69,10 +69,23 @@ RM		= rm -f
 LN		= ln -sf
 INSTALL_PROGRAM	= install
 
+# $(call TEST_CC_OPTION,option,fallback)
+# Test if the C compiler supports the option.
+# Evaluates to "option" if yes, and "fallback" otherwise.
+TEST_CC_OPTION = $(shell \
+	if $(CC) -o /dev/null -c "$(1)" -xc - <<<'int main(void){return 0;}' &>/dev/null; \
+	then \
+		echo "$(1)"; \
+	else \
+		echo "$(2)"; \
+	fi)
+
+STACKPROT = $(call TEST_CC_OPTION,-fstack-protector-strong,-fstack-protector)
+
 OPTFLAGS	= -O2 -g -pipe -Wall -Wextra -Wformat=2 -Werror=implicit-int \
 		  -Werror=implicit-function-declaration -Werror=format-security \
 		  -Wno-sign-compare -Wno-unused-parameter -Wno-clobbered \
-		  -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong \
+		  -Wp,-D_FORTIFY_SOURCE=2 $(STACKPROT) \
 		  --param=ssp-buffer-size=4
 
 CFLAGS		= $(OPTFLAGS) -fPIC -DLIB_STRING=\"${LIB}\" -DRUN_DIR=\"${RUN}\"
-- 
2.12.2

Re: [PATCH 2/2] multipath-tools: fix compilation with gcc < 4.9

[Bart Van Assche]

On Wed, 2017-05-17 at 15:54 +0200, Martin Wilck wrote:
> +# $(call TEST_CC_OPTION,option,fallback)
> +# Test if the C compiler supports the option.
> +# Evaluates to "option" if yes, and "fallback" otherwise.
> +TEST_CC_OPTION = $(shell \
> +	if $(CC) -o /dev/null -c "$(1)" -xc - <<<'int main(void){return 0;}' &>/dev/null; \
> +	then \
> +		echo "$(1)"; \
> +	else \
> +		echo "$(2)"; \
> +	fi)

Since '<<<' is nonstandard, please consider to use 'echo' and a pipe instead.
See also http://pubs.opengroup.org/onlinepubs/9699919799/.

> +STACKPROT = $(call TEST_CC_OPTION,-fstack-protector-strong,-fstack-protector)

Please consider to use ":=" instead of "=" such that TEST_CC_OPTION gets called
once per "make" invocation instead of once per C file that is built.

Thanks,

Bart.