Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kernel fails to build on alpine linux with fortify-headers-2.3.1 #62

Open
ncopa opened this issue Jul 10, 2024 · 26 comments
Open

kernel fails to build on alpine linux with fortify-headers-2.3.1 #62

ncopa opened this issue Jul 10, 2024 · 26 comments
Assignees

Comments

@ncopa
Copy link
Contributor

ncopa commented Jul 10, 2024

In file included from exec-cmd.c:9:
In function 'vsnprintf',
    inlined from 'report.constprop' at subcmd-util.h:13:2:
/usr/include/fortify/stdio.h:162:16: error: 'msg' may be used uninitialized [-Werror=maybe-uninitialized]
  162 |         return __orig_vsnprintf(__s, __n, __f, __v);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/fortify/strings.h:23,
                 from /usr/include/string.h:59,
                 from /usr/include/fortify/string.h:23,
                 from /home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/include/linux/string.h:6,
                 from exec-cmd.c:3:
/usr/include/fortify/stdio.h: In function 'report.constprop':
/usr/include/fortify/stdio.h:152:1: note: in a call to '__orig_vsnprintf' declared with attribute 'access (read_write, 1, 2)' here
  152 | _FORTIFY_FN(vsnprintf) int vsnprintf(char * _FORTIFY_POS0 __s, size_t __n,
      | ^~~~~~~~~~~
In file included from exec-cmd.c:10:
subcmd-util.h:12:14: note: 'msg' declared here
   12 |         char msg[1024];
      |              ^~~
cc1: all warnings being treated as errors
make[5]: *** [/home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/build/Makefile.build:98: /home/ncopa/aports/main/linux-lts/src/build-virt.x86_64/tools/objtool/libsubcmd/exec-cmd.o] Error 1
make[4]: *** [Makefile:80: /home/ncopa/aports/main/linux-lts/src/build-virt.x86_64/tools/objtool/libsubcmd/libsubcmd-in.o] Error 2
make[3]: *** [Makefile:78: /home/ncopa/aports/main/linux-lts/src/build-virt.x86_64/tools/objtool/libsubcmd/libsubcmd.a] Error 2
make[2]: *** [Makefile:73: objtool] Error 2
make[1]: *** [/home/ncopa/aports/main/linux-lts/src/linux-6.6/Makefile:1362: tools/objtool] Error 2
make: *** [/home/ncopa/aports/main/linux-lts/src/linux-6.6/Makefile:234: __sub-make] Error 2
@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

This is the code it complains about:

static inline void report(const char *prefix, const char *err, va_list params)
{
	char msg[1024];
	vsnprintf(msg, sizeof(msg), err, params);
	fprintf(stderr, " %s%s\n", prefix, msg);
}

So compiler complains that msg may be uninitialized when initializing it.

Changing it to:

	char msg[1024] = "";

makes it pass, but I don't think that is a good solution.

@jvoisin
Copy link
Owner

jvoisin commented Jul 10, 2024

Good catch, this is indeed a copy-pasta typo. It should be fixed by 6f54232

@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

Seems to solve it. Thank you for a super fast fix!

algitbot pushed a commit to alpinelinux/aports that referenced this issue Jul 10, 2024
@jvoisin
Copy link
Owner

jvoisin commented Jul 10, 2024

Thank you for making Alpine Linux a willing guinea pig :)

@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

Seems to also affect snprintf:

  DESCEND objtool
In file included from /home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/include/linux/string.h:6,
                 from parse-options.c:3:
In function 'strncpy',
    inlined from 'get_value' at parse-options.c:138:4:
/usr/include/fortify/string.h:327:16: error: '__orig_strncpy' reading 128 bytes from a region of size 17 [-Werror=stringop-overread]
  327 |         return __orig_strncpy(__d, __s, __n);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/fortify/strings.h:23,
                 from /usr/include/string.h:59,
                 from /usr/include/fortify/string.h:23:
/usr/include/fortify/string.h: In function 'get_value':
/usr/include/fortify/string.h:311:1: note: in a call to function '__orig_strncpy' declared with attribute 'access (read_only, 2, 3)'
  311 | _FORTIFY_FN(strncpy) char *strncpy(char * _FORTIFY_POS0 __d,
      | ^~~~~~~~~~~
In file included from parse-options.c:5:
In function 'snprintf',
    inlined from 'get_value' at parse-options.c:89:5:
/usr/include/fortify/stdio.h:284:16: error: 'msg' may be used uninitialized [-Werror=maybe-uninitialized]
  284 |         return __orig_snprintf(__s, __n, __f, __builtin_va_arg_pack());
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/fortify/stdio.h: In function 'get_value':
/usr/include/fortify/stdio.h:274:1: note: in a call to '__orig_snprintf' declared with attribute 'access (read_write, 1, 2)' here
  274 | _FORTIFY_FN(snprintf) int snprintf(char *__s, size_t __n,
      | ^~~~~~~~~~~
parse-options.c:85:30: note: 'msg' declared here
   85 |                         char msg[128];
      |                              ^~~
In function 'snprintf',
    inlined from 'get_value' at parse-options.c:92:5:
/usr/include/fortify/stdio.h:284:16: error: 'msg' may be used uninitialized [-Werror=maybe-uninitialized]
  284 |         return __orig_snprintf(__s, __n, __f, __builtin_va_arg_pack());
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/fortify/stdio.h: In function 'get_value':
/usr/include/fortify/stdio.h:274:1: note: in a call to '__orig_snprintf' declared with attribute 'access (read_write, 1, 2)' here
  274 | _FORTIFY_FN(snprintf) int snprintf(char *__s, size_t __n,
      | ^~~~~~~~~~~
parse-options.c:85:30: note: 'msg' declared here
   85 |                         char msg[128];
      |                              ^~~
In function 'snprintf',
    inlined from 'get_value' at parse-options.c:130:9:
/usr/include/fortify/stdio.h:284:16: error: 'reason' may be used uninitialized [-Werror=maybe-uninitialized]
  284 |         return __orig_snprintf(__s, __n, __f, __builtin_va_arg_pack());
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/fortify/stdio.h: In function 'get_value':
/usr/include/fortify/stdio.h:274:1: note: in a call to '__orig_snprintf' declared with attribute 'access (read_write, 1, 2)' here
  274 | _FORTIFY_FN(snprintf) int snprintf(char *__s, size_t __n,
      | ^~~~~~~~~~~
parse-options.c:127:22: note: 'reason' declared here
  127 |                 char reason[128];
      |                      ^~~~~~

@jvoisin
Copy link
Owner

jvoisin commented Jul 10, 2024

Sigh, I should have been more thorough. Fixed by 9014b02

@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

Not sure if the strncpy should be reported separately? Seems to be another bug.

  CALL    /home/ncopa/aports/main/linux-lts/src/linux-6.6/scripts/checksyscalls.sh
  DESCEND objtool
In file included from /home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/include/linux/string.h:6,
                 from parse-options.c:3:
In function 'strncpy',
    inlined from 'get_value' at parse-options.c:138:4:
/usr/include/fortify/string.h:327:16: error: '__orig_strncpy' reading 128 bytes from a region of size 17 [-Werror=stringop-overread]
  327 |         return __orig_strncpy(__d, __s, __n);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/fortify/strings.h:23,
                 from /usr/include/string.h:59,
                 from /usr/include/fortify/string.h:23:
/usr/include/fortify/string.h: In function 'get_value':
/usr/include/fortify/string.h:311:1: note: in a call to function '__orig_strncpy' declared with attribute 'access (read_only, 2, 3)'
  311 | _FORTIFY_FN(strncpy) char *strncpy(char * _FORTIFY_POS0 __d,
      | ^~~~~~~~~~~
cc1: all warnings being treated as errors

The code it comes from looks legit?

	if (opt->flags & PARSE_OPT_NOBUILD) {
		char reason[128];
		bool noarg = false;

		err = snprintf(reason, sizeof(reason),
				opt->flags & PARSE_OPT_CANSKIP ?
					"is being ignored because %s " :
					"is not available because %s",
				opt->build_opt);
		reason[sizeof(reason) - 1] = '\0';

		if (err < 0)
			strncpy(reason, opt->flags & PARSE_OPT_CANSKIP ?
					"is being ignored" :
					"is not available",
					sizeof(reason));

@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

Maybe something like this? It will not read more than max_len_s:

diff --git a/include/string.h b/include/string.h
index c317b1e..0347ddf 100644
--- a/include/string.h
+++ b/include/string.h
@@ -324,7 +324,7 @@ _FORTIFY_FN(strncpy) char *strncpy(char * _FORTIFY_POS0 __d,
        if (__n > __b)
                __builtin_trap();
 
-       return __orig_strncpy(__d, __s, __n);
+       return __orig_strncpy(__d, __s, max_len_s);
 #endif
 }
 

@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

And with the above fix applied it continues with:

  DESCEND objtool
rm -f /home/ncopa/aports/main/linux-lts/src/build-lts.x86_64/tools/objtool/libsubcmd/libsubcmd.a && ar rcs /home/ncopa/aports/main/linux-lts/src/build-lts.x86_64/tools/objtool/libsubcmd/libsubcmd.a /home/ncopa/aports/main/linux-lts/src/build-lts.x86_64/tools/objtool/libsubcmd/libsubcmd-in.o
make[4]: 'install_headers' is up to date.
In file included from /home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/include/linux/panic.h:6,
                 from /home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/include/linux/kernel.h:11,
                 from /home/ncopa/aports/main/linux-lts/src/build-lts.x86_64/tools/objtool/libsubcmd/include/subcmd/parse-options.h:5,
                 from /home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/objtool/include/objtool/builtin.h:8,
                 from check.c:11:
In function 'sprintf',
    inlined from 'offstr' at /home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/objtool/include/objtool/warn.h:33:9:
/usr/include/fortify/stdio.h:300:23: error: '__orig_snprintf' specified size 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  300 |                 __r = __orig_snprintf(__s, __b, __f, __builtin_va_arg_pack());
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/fortify/strings.h:23,
                 from /usr/include/string.h:59,
                 from /usr/include/fortify/string.h:23,
                 from check.c:6:
/usr/include/fortify/stdio.h: In function 'offstr':
/usr/include/fortify/stdio.h:274:1: note: in a call to function '__orig_snprintf' declared with attribute 'access (read_only, 3)'
  274 | _FORTIFY_FN(snprintf) int snprintf(char *__s, size_t __n,
      | ^~~~~~~~~~~
In function 'sprintf',
    inlined from 'offstr' at /home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/objtool/include/objtool/warn.h:35:4:
/usr/include/fortify/stdio.h:300:23: error: '__orig_snprintf' specified size 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  300 |                 __r = __orig_snprintf(__s, __b, __f, __builtin_va_arg_pack());
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/fortify/stdio.h: In function 'offstr':
/usr/include/fortify/stdio.h:274:1: note: in a call to function '__orig_snprintf' declared with attribute 'access (read_only, 3)'
  274 | _FORTIFY_FN(snprintf) int snprintf(char *__s, size_t __n,
      | ^~~~~~~~~~~
In function 'sprintf',
    inlined from 'offstr' at /home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/objtool/include/objtool/warn.h:38:3:
/usr/include/fortify/stdio.h:300:23: error: '__orig_snprintf' specified size 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  300 |                 __r = __orig_snprintf(__s, __b, __f, __builtin_va_arg_pack());
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/fortify/stdio.h: In function 'offstr':
/usr/include/fortify/stdio.h:274:1: note: in a call to function '__orig_snprintf' declared with attribute 'access (read_only, 3)'
  274 | _FORTIFY_FN(snprintf) int snprintf(char *__s, size_t __n,
      | ^~~~~~~~~~~
In function 'sprintf',
    inlined from 'disas_warned_funcs' at check.c:4624:5,
    inlined from 'check' at check.c:4814:3:
/usr/include/fortify/stdio.h:300:23: error: '__orig_snprintf' specified size 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  300 |                 __r = __orig_snprintf(__s, __b, __f, __builtin_va_arg_pack());
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/fortify/stdio.h: In function 'check':
/usr/include/fortify/stdio.h:274:1: note: in a call to function '__orig_snprintf' declared with attribute 'access (read_only, 3)'
  274 | _FORTIFY_FN(snprintf) int snprintf(char *__s, size_t __n,
      | ^~~~~~~~~~~
cc1: all warnings being treated as errors
make[4]: *** [/home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/build/Makefile.build:98: /home/ncopa/aports/main/linux-lts/src/build-lts.x86_64/tools/objtool/check.o] Error 1
make[3]: *** [Makefile:66: /home/ncopa/aports/main/linux-lts/src/build-lts.x86_64/tools/objtool/objtool-in.o] Error 2
make[2]: *** [Makefile:73: objtool] Error 2
make[1]: *** [/home/ncopa/aports/main/linux-lts/src/linux-6.6/Makefile:1362: tools/objtool] Error 2
make: *** [/home/ncopa/aports/main/linux-lts/src/linux-6.6/Makefile:234: __sub-make] Error 2

The offstr looks like:

static inline char *offstr(struct section *sec, unsigned long offset)
{
	bool is_text = (sec->sh.sh_flags & SHF_EXECINSTR);
	struct symbol *sym = NULL;
	char *str;
	int len;

	if (is_text)
		sym = find_func_containing(sec, offset);
	if (!sym)
		sym = find_symbol_containing(sec, offset);

	if (sym) {
		str = malloc(strlen(sym->name) + strlen(sec->name) + 40);
		len = sprintf(str, "%s+0x%lx", sym->name, offset - sym->offset);
		if (opts.sec_address)
			sprintf(str+len, " (%s+0x%lx)", sec->name, offset);
	} else {
		str = malloc(strlen(sec->name) + 20);
		sprintf(str, "%s+0x%lx", sec->name, offset);
	}

	return str;
}

@jvoisin
Copy link
Owner

jvoisin commented Jul 10, 2024

/usr/include/fortify/stdio.h:300:23: error: '__orig_snprintf' specified size 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]

Is a bit confusing, since 18446744073709551615 is (size_t)-1 on 64b, and the code is:

  if (__b != (__fh_size_t)-1) {
          __r = __orig_snprintf(__s, __b, __f, __builtin_va_arg_pack());

@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

Weird indeed. It also happens with this:

        if (__b != (__fh_size_t)-1 && __b != (__fh_size_t)18446744073709551615) {
                __r = __orig_snprintf(__s, __b, __f, __builtin_va_arg_pack());

@jvoisin
Copy link
Owner

jvoisin commented Jul 10, 2024

#define __fh_size_t __typeof__(sizeof(char)) is size_t on my machine, so I'm really confused :/

@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

I confirmed it is 8 bytes here, on my x86_64.

@markus-oberhumer
Copy link

markus-oberhumer commented Jul 10, 2024

FYI, the type of sizeof(...) always is size_t.

(Edit: just passing by because our CI build on alpine:edge started to fail because of false fortify positives).

@jvoisin
Copy link
Owner

jvoisin commented Jul 10, 2024

So wtf is going on :D

@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

I have tried this to confirm that it actually is __b.

 __r = __orig_snprintf(__s, 20000, __f, __builtin_va_arg_pack());

It makes it build the objtool.

So it must be __b that is the problem.

Then I tried with:

        if (__b < 9223372036854775807) {
                __r = __orig_snprintf(__s, __b, __f, __builtin_va_arg_pack());

But it still triggers the problem. I have no clue what is going on here.

@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

(Edit: just passing by because our CI build on alpine:edge started to fail because of false fortify positives).

@markus-oberhumer is it the same error, or something else?

I think we should try create a smaller test case where we can debug this.

@markus-oberhumer
Copy link

@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

@ncopa Looks like we hit strncpy

https://github.com/upx/upx-test-build-with-zig/actions/runs/9874643415/job/27269509241

I'll push the fix for that, while we scratch our heads.

algitbot pushed a commit to alpinelinux/aports that referenced this issue Jul 10, 2024
@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

This makes build pass:

    if (__b < 1) {
                __r = __orig_snprintf(__s, __b, __f, __builtin_va_arg_pack());

This fails with same error:

        if (__b < 2) {
                __r = __orig_snprintf(__s, __b, __f, __builtin_va_arg_pack());

@t8m
Copy link

t8m commented Jul 10, 2024

Try building openssl - it will also fail to build with multiple false positive warnings if -Werror is used.

@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

I have a smallish reproducer:

#include <stdio.h>

static char *offstr(char *str)
{
	int len = 0;

	len = sprintf(str, "%s+0x%lx", "foo", (long unsigned int)0);
	sprintf(str+len, " (%s+0x%lx)","bar", (long unsigned int)0);
	if (len < 0)
		return NULL;
	return str;
}

int main() {
	char buf[100];
	char *c = offstr(buf);
	printf("%s\n", c);
	return 0;
}

algitbot pushed a commit to alpinelinux/aports that referenced this issue Jul 10, 2024
Still have a nasty issue:
jvoisin/fortify-headers#62 (comment)

Revert again til we have this sorted out.

This reverts commit ccebbed.
@ncopa
Copy link
Contributor Author

ncopa commented Jul 10, 2024

I have reverted the fortify-headers upgrade in Alpine Linux til we have this sorted.

bell-sw pushed a commit to bell-sw/alpaquita-aports that referenced this issue Jul 12, 2024
[ commit d18d03848c08036fb33ae85a921bbac9bc3a0864 ]

upstream: jvoisin/fortify-headers#62
bell-sw pushed a commit to bell-sw/alpaquita-aports that referenced this issue Jul 12, 2024
[ commit 2c820d02fa147df0b4cf2cab2b01eb2063aa794b ]

fix for snprintf and strncpy.

ref: jvoisin/fortify-headers#62
bell-sw pushed a commit to bell-sw/alpaquita-aports that referenced this issue Jul 12, 2024
[ commit 2fcdb47598f36a0b1dff5cfa468628be2b838da7 ]

Still have a nasty issue:
jvoisin/fortify-headers#62 (comment)

Revert again til we have this sorted out.

This reverts commit ccebbedc0f06db6e3ab86a0e6864fa1b413b6002.
@markus-oberhumer
Copy link

@jvoisin Could you please create a single-file reproducer on Compiler Explorer https://godbolt.org/ ?

kholmanskikh pushed a commit to bell-sw/alpaquita-aports that referenced this issue Jul 30, 2024
[ commit d18d03848c08036fb33ae85a921bbac9bc3a0864 ]

upstream: jvoisin/fortify-headers#62
kholmanskikh pushed a commit to bell-sw/alpaquita-aports that referenced this issue Jul 30, 2024
[ commit 2c820d02fa147df0b4cf2cab2b01eb2063aa794b ]

fix for snprintf and strncpy.

ref: jvoisin/fortify-headers#62
kholmanskikh pushed a commit to bell-sw/alpaquita-aports that referenced this issue Jul 30, 2024
[ commit 2fcdb47598f36a0b1dff5cfa468628be2b838da7 ]

Still have a nasty issue:
jvoisin/fortify-headers#62 (comment)

Revert again til we have this sorted out.

This reverts commit ccebbedc0f06db6e3ab86a0e6864fa1b413b6002.
@jvoisin
Copy link
Owner

jvoisin commented Sep 6, 2024

f2e7f24 should work around the issue, as I really don't want to dive into gcc's code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants