]> git.ipfire.org Git - thirdparty/openvpn.git/commitdiff
sitnl: replace NLMSG_TAIL macro with noinline function
authorAntonio Quartulli <antonio@mandelbit.com>
Wed, 6 Nov 2024 13:17:03 +0000 (14:17 +0100)
committerGert Doering <gert@greenie.muc.de>
Sat, 9 Nov 2024 11:23:14 +0000 (12:23 +0100)
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 <antonio@mandelbit.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20241106131705.11069-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29710.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
src/openvpn/networking_sitnl.c

index f53f5ee9ebf43525cfaac7cf7126717e153ea7a4..6b750e86816278f68a6b0feda9f70abc6e861212 100644 (file)
         }                                                           \
     }
 
-#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
@@ -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;