platform,core: merge branch 'th/platform-struct-packing'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1217

4 files changed, 72 insertions, 59 deletions

diff --git a/src/core/nm-audit-manager.c b/src/core/nm-audit-manager.c
index dd96d834e5..4e134d1a3c 100644
--- a/src/core/nm-audit-manager.c
+++ b/src/core/nm-audit-manager.c
@@ -135,10 +135,13 @@ build_message(NMStrBuf *strbuf, AuditBackend backend, GPtrArray *fields)
 #if HAVE_LIBAUDIT
             if (backend == BACKEND_AUDITD) {
                 if (field->need_encoding) {
-                    gs_free char *value = NULL;
+                    nm_auto_free char *value = NULL;
 
                     value = audit_encode_nv_string(field->name, str, 0);
-                    nm_str_buf_append(strbuf, value);
+                    if (value)
+                        nm_str_buf_append(strbuf, value);
+                    else
+                        nm_str_buf_append_printf(strbuf, "%s=???", field->name);
                 } else
                     nm_str_buf_append_printf(strbuf, "%s=%s", field->name, str);
                 continue;
diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c
index 50512ee10f..b8102cc8bc 100644
--- a/src/libnm-glib-aux/nm-shared-utils.c
+++ b/src/libnm-glib-aux/nm-shared-utils.c
@@ -37,6 +37,13 @@ const void *const _NM_PTRARRAY_EMPTY[1] = {NULL};
 
 const NMIPAddr nm_ip_addr_zero = {};
 
+/* We use _nm_alignas(NMIPAddr). Ensure that this struct has the same
+ * alignment as in_addr_t and struct in6_addr. */
+G_STATIC_ASSERT(_nm_alignof(NMIPAddr) == 4);
+G_STATIC_ASSERT(_nm_alignof(in_addr_t) == 4);
+G_STATIC_ASSERT(_nm_alignof(struct in_addr) == 4);
+G_STATIC_ASSERT(_nm_alignof(struct in6_addr) == 4);
+
 /* this initializes a struct in_addr/in6_addr and allows for untrusted
  * arguments (like unsuitable @addr_family or @src_len). It's almost safe
  * in the sense that it verifies input arguments strictly. Also, it
diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h
index 80f6bcddb9..0e123cf1c0 100644
--- a/src/libnm-glib-aux/nm-shared-utils.h
+++ b/src/libnm-glib-aux/nm-shared-utils.h
@@ -216,7 +216,7 @@ nm_ether_addr_equal(const NMEtherAddr *a, const NMEtherAddr *b)
 
 typedef struct {
     union {
-        guint8          addr_ptr[1];
+        guint8          addr_ptr[sizeof(struct in6_addr)];
         in_addr_t       addr4;
         struct in_addr  addr4_struct;
         struct in6_addr addr6;
diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h
index 7113607e88..a4cba9ced8 100644
--- a/src/libnm-platform/nm-platform.h
+++ b/src/libnm-platform/nm-platform.h
@@ -333,6 +333,9 @@ typedef enum {
      * should be configured. */             \
     bool a_force_commit : 1;                                                                 \
                                                                                              \
+    /* Don't have a bitfield as last field in __NMPlatformIPAddress_COMMON. It would then
+     * be unclear how the following fields get merged. We could also use a zero bitfield,
+     * but instead we just have there the uint8 field. */    \
     guint8 plen;                                                                             \
     ;
 
@@ -343,10 +346,7 @@ typedef enum {
  **/
 typedef struct {
     __NMPlatformIPAddress_COMMON;
-    union {
-        guint8  address_ptr[1];
-        guint32 __dummy_for_32bit_alignment;
-    };
+    _nm_alignas(NMIPAddr) guint8 address_ptr[];
 } NMPlatformIPAddress;
 
 /**
@@ -358,11 +358,15 @@ struct _NMPlatformIP4Address {
 
     /* Whether the address is ready to be configured. By default, an address is, but this
      * flag may indicate that the address is just for tracking purpose only, but the ACD
-     * state is not yet ready for the address to be configured. */
+     * state is not yet ready for the address to be configured.
+     *
+     * This bit fits actually in an alignment gap between __NMPlatformIPAddress_COMMON and
+     * "address" field. Usually "address" must be the first field after __NMPlatformIPAddress_COMMON,
+     * but there is a gap. We have a static assertion that checks this, so all is good. */
     bool a_acd_not_ready : 1;
 
     /* The local address IFA_LOCAL. */
-    in_addr_t address;
+    _nm_alignas(NMIPAddr) in_addr_t address;
 
     /* The IFA_ADDRESS PTP peer address. This field is rather important, because
      * it constitutes the identifier for the IPv4 address (e.g. you can add two
@@ -390,7 +394,7 @@ struct _NMPlatformIP4Address {
  **/
 struct _NMPlatformIP6Address {
     __NMPlatformIPAddress_COMMON;
-    struct in6_addr address;
+    _nm_alignas(NMIPAddr) struct in6_addr address;
     struct in6_addr peer_address;
 };
 
@@ -426,6 +430,47 @@ typedef union {
 #define __NMPlatformIPRoute_COMMON                                                        \
     __NMPlatformObjWithIfindex_COMMON;                                                    \
                                                                                           \
+    /* rtnh_flags
+     *
+     * Routes with rtm_flags RTM_F_CLONED are hidden by platform and
+     * do not exist from the point-of-view of platform users.
+     * Such a route is not alive, according to nmp_object_is_alive().
+     *
+     * NOTE: currently we ignore all flags except RTM_F_CLONED
+     * and RTNH_F_ONLINK.
+     * We also may not properly consider the flags as part of the ID
+     * in route-cmp. */                                                                         \
+    unsigned r_rtm_flags;                                                                 \
+                                                                                          \
+    /* RTA_METRICS.RTAX_ADVMSS (iproute2: advmss) */                                      \
+    guint32 mss;                                                                          \
+                                                                                          \
+    /* RTA_METRICS.RTAX_WINDOW (iproute2: window) */                                      \
+    guint32 window;                                                                       \
+                                                                                          \
+    /* RTA_METRICS.RTAX_CWND (iproute2: cwnd) */                                          \
+    guint32 cwnd;                                                                         \
+                                                                                          \
+    /* RTA_METRICS.RTAX_INITCWND (iproute2: initcwnd) */                                  \
+    guint32 initcwnd;                                                                     \
+                                                                                          \
+    /* RTA_METRICS.RTAX_INITRWND (iproute2: initrwnd) */                                  \
+    guint32 initrwnd;                                                                     \
+                                                                                          \
+    /* RTA_METRICS.RTAX_MTU (iproute2: mtu) */                                            \
+    guint32 mtu;                                                                          \
+                                                                                          \
+    /* RTA_PRIORITY (iproute2: metric)
+     * If "metric_any" is %TRUE, then this is interpreted as an offset that will be
+     * added to a default base metric. In such cases, the offset is usually zero. */                                                    \
+    guint32 metric;                                                                       \
+                                                                                          \
+    /* rtm_table, RTA_TABLE.
+     *
+     * This is not the original table ID. Instead, 254 (RT_TABLE_MAIN) and
+     * zero (RT_TABLE_UNSPEC) are swapped, so that the default is the main
+     * table. Use nm_platform_route_table_coerce()/nm_platform_route_table_uncoerce(). */                                                              \
+    guint32 table_coerced;                                                                \
     /* The NMIPConfigSource. For routes that we receive from cache this corresponds
      * to the rtm_protocol field (and is one of the NM_IP_CONFIG_SOURCE_RTPROT_* values).
      * When adding a route, the source will be coerced to the protocol using
@@ -440,8 +485,6 @@ typedef union {
      * to zero, in which case the first matching route (with proto ignored) is deleted. */       \
     NMIPConfigSource rt_source;                                                           \
                                                                                           \
-    guint8 plen;                                                                          \
-                                                                                          \
     /* RTA_METRICS:
      *
      * For IPv4 routes, these properties are part of their
@@ -480,62 +523,22 @@ typedef union {
     /* Whether the route should be committed even if it was removed externally. */        \
     bool r_force_commit : 1;                                                              \
                                                                                           \
-    /* rtnh_flags
-     *
-     * Routes with rtm_flags RTM_F_CLONED are hidden by platform and
-     * do not exist from the point-of-view of platform users.
-     * Such a route is not alive, according to nmp_object_is_alive().
-     *
-     * NOTE: currently we ignore all flags except RTM_F_CLONED
-     * and RTNH_F_ONLINK.
-     * We also may not properly consider the flags as part of the ID
-     * in route-cmp. */                                                                         \
-    unsigned r_rtm_flags;                                                                 \
-                                                                                          \
-    /* RTA_METRICS.RTAX_ADVMSS (iproute2: advmss) */                                      \
-    guint32 mss;                                                                          \
-                                                                                          \
-    /* RTA_METRICS.RTAX_WINDOW (iproute2: window) */                                      \
-    guint32 window;                                                                       \
-                                                                                          \
-    /* RTA_METRICS.RTAX_CWND (iproute2: cwnd) */                                          \
-    guint32 cwnd;                                                                         \
-                                                                                          \
-    /* RTA_METRICS.RTAX_INITCWND (iproute2: initcwnd) */                                  \
-    guint32 initcwnd;                                                                     \
-                                                                                          \
-    /* RTA_METRICS.RTAX_INITRWND (iproute2: initrwnd) */                                  \
-    guint32 initrwnd;                                                                     \
-                                                                                          \
-    /* RTA_METRICS.RTAX_MTU (iproute2: mtu) */                                            \
-    guint32 mtu;                                                                          \
-                                                                                          \
-    /* RTA_PRIORITY (iproute2: metric)
-     * If "metric_any" is %TRUE, then this is interpreted as an offset that will be
-     * added to a default base metric. In such cases, the offset is usually zero. */                                                    \
-    guint32 metric;                                                                       \
-                                                                                          \
-    /* rtm_table, RTA_TABLE.
-     *
-     * This is not the original table ID. Instead, 254 (RT_TABLE_MAIN) and
-     * zero (RT_TABLE_UNSPEC) are swapped, so that the default is the main
-     * table. Use nm_platform_route_table_coerce()/nm_platform_route_table_uncoerce(). */                                                              \
-    guint32 table_coerced;                                                                \
-                                                                                          \
     /* rtm_type.
      *
      * This is not the original type, if type_coerced is 0 then
      * it means RTN_UNSPEC otherwise the type value is preserved.
-     * */                                                                          \
+     */                                                                          \
     guint8 type_coerced;                                                                  \
+                                                                                          \
+    /* Don't have a bitfield as last field in __NMPlatformIPAddress_COMMON. It would then
+     * be unclear how the following fields get merged. We could also use a zero bitfield,
+     * but instead we just have there the uint8 field. */ \
+    guint8 plen;                                                                          \
     ;
 
 typedef struct {
     __NMPlatformIPRoute_COMMON;
-    union {
-        guint8  network_ptr[1];
-        guint32 __dummy_for_32bit_alignment;
-    };
+    _nm_alignas(NMIPAddr) guint8 network_ptr[];
 } NMPlatformIPRoute;
 
 #define NM_PLATFORM_IP_ROUTE_CAST(route) \