coova / coova-chilli

CoovaChilli is an open-source software access controller for captive portal hotspots.
Other
514 stars 257 forks source link

Make error on Ubuntu 20.04 may result in an unaligned pointer value #521

Closed nullsoft8411 closed 3 years ago

nullsoft8411 commented 3 years ago

Here is the error that i get:

libtool: compile: gcc -DHAVE_CONFIG_H -I. -I.. -I../json -D_GNU_SOURCE -Wall -Werror -fno-builtin -fno-strict-aliasing -fomit-frame-pointer -funroll-loops -pipe -I../bstring -DDEFCHILLICONF=\"/etc/chilli.conf\" -DDEFPIDFILE=\"/var/run/chilli.pid\" -DDEFSTATEDIR=\"/var/run\" -DSBINDIR=\"/sbin\" -g -O2 -MT chilli.lo -MD -MP -MF .deps/chilli.Tpo -c chilli.c -fPIC -DPIC -o .libs/chilli.o chilli.c: In function ‘leaky_bucket’: chilli.c:647:30: error: taking address of packed member of ‘struct session_state’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 647 | timediff = mainclock_diffd(&conn->s_state.last_bw_time); | ^~~~~~~ chilli.c: In function ‘uam_msg’: chilli.c:6077:34: error: taking address of packed member of ‘struct redir_msg_data’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 6077 | if (ippool_getip(ippool, &ipm, &msg->mdata.address.sin_addr)) { | ^~~~~~~~ chilli.c: In function ‘find_app_conn’: chilli.c:6201:38: error: taking address of packed member of ‘struct cmdsock_request’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 6201 | appconn = dhcp_get_appconn_ip(0, &req->ip); | ^~~~ chilli.c: In function ‘redir_msg’: chilli.c:7125:10: error: taking address of packed member of ‘struct redir_msg_data’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 7125 | &msg.mdata.address, | ^~~~~~ chilli.c:7126:10: error: taking address of packed member of ‘struct redir_msg_data’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 7126 | &msg.mdata.baddress, | ^~~~~~~ In file included from /usr/include/string.h:495, from system.h:31, from chilli.h:24, from chilli.c:21: In function ‘strncpy’, inlined from ‘cb_radius_auth_conf’ at chilli.c:4618:4: /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘builtin___strncpy_chk’ output may be truncated copying between 0 and 29 bytes from a string of length 252 [-Werror=stringop-truncation] 106 | return builtin_strncpy_chk (dest, src, len, bos (dest)); | ^~~~~~~~~~~~~~ cc1: all warnings being treated as errors make[3]: [Makefile:968: chilli.lo] Error 1 make[3]: Leaving directory '/tmp/coova-chilli-1.5/src' make[2]: [Makefile:1011: all-recursive] Error 1 make[2]: Leaving directory '/tmp/coova-chilli-1.5/src' make[1]: [Makefile:417: all-recursive] Error 1 make[1]: Leaving directory '/tmp/coova-chilli-1.5' make: [Makefile:345: all] Error 2 root@mail:/tmp/coova-chilli-1.5#

`

can someone help me to fix this please

BR, Sahin

gcavelier commented 3 years ago

Had the same issue.

As a workaround, you can disable the '-Werror' flag which is causing this by modifying the AM_CFLAGS variable in src/Makefile

I replaced (line 636) \: AM_CFLAGS = -D_GNU_SOURCE -Wall -Werror -fno-builtin \ with AM_CFLAGS = -D_GNU_SOURCE -Wall -fno-builtin \

and 'make' now issues some warnings but is compiling fine.

aleksander0m commented 3 years ago

The whole codebase has this issue in multiple places; I was hoping there would be a few places to fix, but they're truly a ton. Looks like the packed structs are used for multiple things, and the code just assumes it can reference the address of the members of the struct, which shouldn't be done as the alignment of the pointers would be compromised.

If keeping the structs packed is truly mandatory, these cases can be fixed like this (e.g. in this case below last_bw_time is only being read, not being updated, so no need to update it after the function call):

diff --git a/src/chilli.c b/src/chilli.c
index 0a7ba30..c3d86c0 100644
--- a/src/chilli.c
+++ b/src/chilli.c
@@ -643,8 +643,10 @@ leaky_bucket(struct app_conn_t *conn,
   int result = 0;
   uint64_t upbytes=0, dnbytes=0;
   long double timediff;
+  struct timespec last_bw_time;

-  timediff = mainclock_diffd(&conn->s_state.last_bw_time);
+  last_bw_time = conn->s_state.last_bw_time;
+  timediff = mainclock_diffd(&last_bw_time);

Or we can just close our eyes and remove the -Werror.... (or add -Wno-error=address-of-packed-member)