Skip to content

Commit

Permalink
sitnl: replace NLMSG_TAIL macro with noinline function
Browse files Browse the repository at this point in the history
The NLMSG_TAIL macro is confusing gcc when compiling with -O3, leading
to warnings like:

networking_sitnl.c:143:9: warning: writing 4 bytes into a region of size 0 [-Wstringop-overflow=]
  143 |         memcpy(RTA_DATA(rta), data, alen);
      |         ^
networking_sitnl.c:101:21: note: at offset [72, 88] into destination object ‘n’ of size 16
  101 |     struct nlmsghdr n;
      |                     ^

Replacing the macro with a function is also not effective because gcc
will inline it and get confused again.

The only way out is to write a function that never gets inline'd and
replace the macro with it.

Tested on linux with gcc and clang.

Change-Id: I9306a590a10a7d5cba32abe06d269494fec41ba6
Signed-off-by: Antonio Quartulli <[email protected]>
Acked-by: Frank Lichtenheld <[email protected]>
Message-Id: <[email protected]>
URL: https://www.mail-archive.com/[email protected]/msg29710.html
Signed-off-by: Gert Doering <[email protected]>
  • Loading branch information
ordex authored and cron2 committed Nov 9, 2024
1 parent 7f0214c commit 648e160
Showing 1 changed file with 21 additions and 8 deletions.
29 changes: 21 additions & 8 deletions src/openvpn/networking_sitnl.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,34 @@
} \
}

#define NLMSG_TAIL(nmsg) \
((struct rtattr *)(((uint8_t *)(nmsg)) + NLMSG_ALIGN((nmsg)->nlmsg_len)))

#define SITNL_NEST(_msg, _max_size, _attr) \
({ \
struct rtattr *_nest = NLMSG_TAIL(_msg); \
struct rtattr *_nest = sitnl_nlmsg_tail(_msg); \
SITNL_ADDATTR(_msg, _max_size, _attr, NULL, 0); \
_nest; \
})

#define SITNL_NEST_END(_msg, _nest) \
{ \
_nest->rta_len = (void *)NLMSG_TAIL(_msg) - (void *)_nest; \
#define SITNL_NEST_END(_msg, _nest) \
{ \
_nest->rta_len = (void *)sitnl_nlmsg_tail(_msg) - (void *)_nest; \
}

/* This function was originally implemented as a macro, but compiling with
* gcc and -O3 was getting confused about the math and thus raising
* security warnings on subsequent memcpy() calls.
*
* Converting the macro to a function was not enough, because gcc was still
* inlining it and falling in the same math trap.
*
* The only way out to avoid any warning/error is to force the function to
* not be inline'd.
*/
static __attribute__ ((noinline)) void *
sitnl_nlmsg_tail(const struct nlmsghdr *nlh)
{
return (unsigned char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len);
}

/**
* Generic address data structure used to pass addresses and prefixes as
* argument to AF family agnostic functions
Expand Down Expand Up @@ -130,7 +143,7 @@ sitnl_addattr(struct nlmsghdr *n, int maxlen, int type, const void *data,
return -EMSGSIZE;
}

rta = NLMSG_TAIL(n);
rta = sitnl_nlmsg_tail(n);
rta->rta_type = type;
rta->rta_len = len;

Expand Down

0 comments on commit 648e160

Please sign in to comment.