Closed Marcool04 closed 1 week ago
Hi @Marcool04,
thanks for this detailed report.
Even tough this is a bug, but may be some CFLAG/C restriction blocks the successful build.
I also get this message but not as an error, but "just" a warning:
config.c: In function ‘process_command_config’:
config.c:1107:32: warning: passing argument 1 of ‘apr_filepath_root’ from incompatible pointer type [-Wincompatible-pointer-types]
1107 | status = apr_filepath_root(&rootpath, &incpath, APR_FILEPATH_TRUENAME | APR_FILEPATH_NATIVE, ptemp);
| ^~~~~~~~~
| |
| char **
In file included from /usr/include/apr-1.0/apr_file_io.h:29,
from /usr/include/apr-1.0/apr_network_io.h:26,
from /usr/include/apr-1.0/apr_buckets.h:29,
from /usr/include/apache2/util_filter.h:26,
from /usr/include/apache2/http_core.h:32,
from config.c:18:
/usr/include/apr-1.0/apr_file_info.h:336:58: note: expected ‘const char **’ but argument is of type ‘char **’
336 | APR_DECLARE(apr_status_t) apr_filepath_root(const char **rootpath,
| ~~~~~~~~~~~~~^~~~~~~~
config.c:1107:43: warning: passing argument 2 of ‘apr_filepath_root’ from incompatible pointer type [-Wincompatible-pointer-types]
1107 | status = apr_filepath_root(&rootpath, &incpath, APR_FILEPATH_TRUENAME | APR_FILEPATH_NATIVE, ptemp);
| ^~~~~~~~
| |
| char **
In file included from /usr/include/apr-1.0/apr_file_io.h:29,
from /usr/include/apr-1.0/apr_network_io.h:26,
from /usr/include/apr-1.0/apr_buckets.h:29,
from /usr/include/apache2/util_filter.h:26,
from /usr/include/apache2/http_core.h:32,
from config.c:18:
/usr/include/apr-1.0/apr_file_info.h:337:58: note: expected ‘const char **’ but argument is of type ‘char **’
337 | const char **filepath,
| ~~~~~~~~~~~~~^~~~~~~~
Unfortunately based on the given data (including your excellent build log) I wasn't able to reproduce this issue. I tried with similar settings (eg. passed -Werror=format-security
to CFLAGS
), but no luck.
Could you review your settings?
Thanks for looking into this @airween , glad the report was clear.
I have this when I run env
just before make
:
SHELL=/bin/bash
BUILDTOOL=devtools
SUDO_GID=0
PYTHONHASHSEED=0
TEXTDOMAINDIR=/usr/share/locale
SUDO_COMMAND=/bin/bash -c bash -c cd\ /startdir;\ makepkg\ "$@" -bash --syncdeps --noconfirm --log --holdver --skipinteg --install
SUDO_USER=root
PWD=/build/libmodsecurity2/src/modsecurity-2.9.7 23:21:55 [793/10743]
SOURCE_DATE_EPOCH=1718659294
LOGNAME=builduser
CXXFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security
-fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -Wp,-D_GLIBCXX_ASSERTIONS -g
-ffile-prefix-map=/build/libmodsecurity2/src=/usr/src/debug/libmodsecurity2 -flto=auto
DEBUG_RUSTFLAGS=-C debuginfo=2 --remap-path-prefix=/build/libmodsecurity2/src=/usr/src/debug/libmodsecurity2
COMMAND_MODE=legacy
LDFLAGS=-Wl,-O1 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,-z,pack-relative-relocs -flto=auto
HOME=/build
LANG=C.UTF-8
RUSTFLAGS=-Cforce-frame-pointers=yes -C debuginfo=2 --remap-path-prefix=/build/libmodsecurity2/src=/usr/src/debug/libmodsecurity2
MAKEFLAGS=-j10
TERM=tmux-256color
USER=builduser
SHLVL=1
DEBUGINFOD_URLS=https://debuginfod.archlinux.org
CHOST=x86_64-pc-linux-gnu
PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl
CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security
-fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/lib
modsecurity2/src=/usr/src/debug/libmodsecurity2 -flto=auto
SUDO_UID=0
MAIL=/var/mail/builduser
BUILDTOOLVER=1:1.2.0-1-any
OLDPWD=/build/libmodsecurity2/src
TEXTDOMAIN=pacman-scripts
_=/usr/bin/env
nothing in there stands out to me... How about you?
I also tried export CFLAGS="${CFLAGS} -Wno-error=incompatible-pointer-types
just before make
and that made no difference...
I'm a bit stuck for ideas here. BTW this is being run via an Arch linux build script that runs the compilation in a clean, up to data chroot. Usually in this context the Arch default build flags for C are pretty mundane.
'rootpath' must indeed be 'const'. Note that 'rootpath' is used later for a totally different treatment - see if (APR_ERELATIVE == status). This should use another non-const variable.
One possible explanation here for this being considered an error on my end and a warning elsewhere could be the fact that Arch uses quite bleeding-edge versions of software?
$ pacman -Si gcc
Repository : core
Name : gcc
Version : 14.1.1+r58+gfc9fb69ad62-1
Description : The GNU Compiler Collection - C and C++ frontends
Architecture : x86_64
[snip]
Build Date : Wed 22 May 2024 02:09:02 PM CEST
A bit more digging reveals this is indeed the case. From https://gcc.gnu.org/gcc-14/changes.html :
C: Certain warnings about are now errors, see Porting to GCC 14 for details.
And there we have:
Type checking on pointer types (-Werror=incompatible-pointer-types) GCC no longer allows implicitly casting all pointer types to all other pointer types. This behavior is now restricted to the void * type and its qualified variations.
To fix compilation errors resulting from that, you can add the appropriate casts, and maybe consider using void in more places (particularly for old programs that predate the introduction of void into the C language).
Programs that do not carefully track pointer types are likely to contain aliasing violations, so consider building with -fno-strict-aliasing. (Whether casts are written manually or performed by GCC automatically does not make a difference in terms of strict aliasing violations.)
A frequent source of incompatible function pointer types involves callback functions that have more specific argument types (or less specific return types) than the function pointer they are assigned to. For example, old code which attempts to sort an array of strings might look like this:
include
include
include
int compare (char a, char b) { return strcmp (a, b); }
void sort (char *array, size_t length) { qsort (array, length, sizeof (array), compare); }
To address this, the callback function should be defined with the correct type, and the arguments should be cast as appropriate before they are used (as calling a function through a function pointer of an incorrect type is undefined):
int compare (const void a1, const void b1) { char const a = a1; char const b = b1; return strcmp (a, b); }
A similar issue can arise with object pointer types. Consider a function that is declared with an argument of type void *, and you want to pass the address of a variable of type int :
extern int *pointer; extern void operate (int command, void **);
operate (0, &pointer);
In these cases, it may be appropriate to make a copy of the pointer with the correct void * type:
extern int *pointer; extern void operate (int command, void **);
void *pointer1 = pointer; operate (0, &pointer1); pointer = pointer1;
As mentioned initially, adding the cast here would not eliminate any strict aliasing violation in the implementation of the operate function. Of course in general, introducing such additional copies may alter program behavior.
Some programming styles rely on implicit casts between related object pointer types to implement C++-style struct inheritance. It may be possible to avoid writing manual casts with the -fplan9-extensions option and unnamed initial struct fields for the base type in derived structs.
Some programs use a concrete function pointer type such as void () (void) as a generic function pointer type (similar to void for object pointer types), and rely on implicit casts to and from this type. The reason for that is that C does not offer a generic function pointer type, and standard C disallows casts between function pointer types and object pointer types. On most targets, GCC supports implicit conversion between void * and function pointer types. However, for a portable solution, the concrete function pointer type needs to be used, together with explicit casts.
Would a cast be sufficient (for both parameters)? @Marcool04 Can you test this?
I've successfully tried the following patch which does allow modsecurity 2 to build on an up to date archlinux gcc14:
diff -ura modsecurity-2.9.7.orig/standalone/config.c modsecurity-2.9.7.mod/standalone/config.c
--- modsecurity-2.9.7.orig/standalone/config.c 2021-06-21 14:34:56.000000000 +0200
+++ modsecurity-2.9.7.mod/standalone/config.c 2024-10-04 07:29:48.346602243 +0200
@@ -1028,7 +1028,8 @@
ap_directive_t *newdir;
int optional;
char *err = NULL;
- char *rootpath, *incpath;
+ const char *rootpath, *incpath;
+ char *configfilepath;
int li;
errmsg = populate_include_files(p, ptemp, ari, filename, 0);
@@ -1108,13 +1109,13 @@
/* we allow APR_SUCCESS and APR_EINCOMPLETE */
if (APR_ERELATIVE == status) {
- rootpath = apr_pstrdup(ptemp, parms->config_file->name);
- li = strlen(rootpath) - 1;
+ configfilepath = apr_pstrdup(ptemp, parms->config_file->name);
+ li = strlen(configfilepath) - 1;
- while(li >= 0 && rootpath[li] != '/' && rootpath[li] != '\\')
- rootpath[li--] = 0;
+ while(li >= 0 && configfilepath[li] != '/' && configfilepath[li] != '\\')
+ configfilepath[li--] = 0;
- w = apr_pstrcat(p, rootpath, w, NULL);
+ w = apr_pstrcat(p, configfilepath, w, NULL);
}
else if (APR_EBADPATH == status) {
ap_cfg_closefile(parms->config_file);
In fact, as I was looking through config.c I noticed that here: https://github.com/owasp-modsecurity/ModSecurity/blob/1121ef0bed4f083eb6de1e7c49daf49ab554a1da/standalone/config.c#L989 rootpath
was correctly cast as a const char
before being passed to apr_filepath_root
and that call didn't generate the error. I've also renamed the other completely unrelated use of rootpath
to configfilepath
but that might not be the best name, I don't know what variable naming principles you like to follow.
LGTM. Do you want to make a PR?
Describe the bug
Building fails with these two errors related to APR:
Logs and dumps
Full build log is here: https://termbin.com/4pis (too big for a github comment)
To Reproduce
The build script is this one: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=libmodsecurity2 equivalent to getting the latest tagged v2 release (v2.9.7 and doing:
Expected behavior
Successful build.
Server (please complete the following information):
Rule Set (please complete the following information): N/A
Additional context
Since the error is coming from the file
/usr/include/apr-1/apr_file_io.h
I checked that out, it is owned by packageapr
, so I checked if any recent changes had occurred there but we have:so this is old stuff...