strncpy stringop-truncation failure

[Corey Bryant]

Our focal package build for Ubuntu ppc64el is failing with:

gcc -DHAVE_CONFIG_H -I. -I../include -I../include -I../include/rs_vand -Wdate-time -D_FORTIFY_SOURCE=2 -O2 -g -Werror -D_GNU_SOURCE=1 -Wall -pedantic -std=c99 -g -O3 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -DARCH_64 -c -o builtin/rs_vand/liberasurecode_rs_vand_test-liberasurecode_rs_vand_test.o `test -f 'builtin/rs_vand/liberasurecode_rs_vand_test.c' || echo './'`builtin/rs_vand/liberasurecode_rs_vand_test.c
In file included from /usr/include/string.h:494,
                 from ../include/erasurecode/erasurecode_stdinc.h:67,
                 from ../include/erasurecode/erasurecode.h:32,
                 from liberasurecode_test.c:32:
In function ‘strncpy’,
    inlined from ‘create_fake_frags_no_meta’ at liberasurecode_test.c:408:9,
    inlined from ‘test_decode_invalid_args’ at liberasurecode_test.c:699:23:
/usr/include/powerpc64le-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output truncated before terminating nul copying 1 byte from a string of the same length [-Werror=stringop-truncation]
  106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

With -Werror=format-security,-Werror=stringop-truncation gets set. -Werror=stringop-truncation will fail the build, in this case because strncpy is copying a string but not copying the string's null terminator (ie. it's only copying one byte of " "). The actual error is occurring in create_fake_frags_no_meta at liberasurecode_test.c:408, but I think it can be fixed in test_decode_invalid_args at liberasurecode_test.c:699.

[Corey Bryant (corey.bryant)]

https://review.opendev.org/#/c/702530/

[Tim Burke (1-tim-z)]

Looks like it it was noticed because create_fake_frags_no_meta got inlined; -Werror=format-security is probably a good idea but not strictly necessary to repro. All I needed to see it locally was

diff --git a/test/liberasurecode_test.c b/test/liberasurecode_test.c
index d3ca200..9955987 100644
--- a/test/liberasurecode_test.c
+++ b/test/liberasurecode_test.c
@@ -385,8 +385,8 @@ int *create_skips_array(struct ec_args *args, int skip)
     return buf;
 }

-static int create_fake_frags_no_meta(char ***array, int num_frags,
- const char *data, int data_len)
+inline static int create_fake_frags_no_meta(char ***array, int num_frags,
+ const char *data, int data_len)
 {
     // N.B. The difference from creat_frags_arry is to creat new
     // memory allocation and set a copy of data/parity there. The

There are only two callers for create_fake_frags_no_meta, and it's all in the test code anyway -- I'm inclined to rework things so we can just use memset which seems a more natural fit. Those fragments are supposed to be slabs of memory and shouldn't be used like normal C strings, ever.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/702530
Committed: https://git.openstack.org/cgit/openstack/liberasurecode/commit/?id=0eb6cd321e312e32b3ad12c34089466eae60cd88
Submitter: Zuul
Branch: master

commit 0eb6cd321e312e32b3ad12c34089466eae60cd88
Author: Corey Bryant <email address hidden>
Date: Tue Jan 14 15:47:44 2020 -0500

    Fix create_fake_frags_no_meta to use memset to fill frags

    These aren't C strings; we shouldn't be using strncpy and friends.

    Change-Id: I50cd7922dfa377ea27f3c9558a8a7268120ec733
    Closes-Bug: #1859710