From: Thomas Haller <thaller@redhat.com>
Date: Fri, 31 May 2024 10:34:28 +0200
Subject: lldp: fix crash dereferencing NULL pointer during debug logging

During nm_lldp_neighbor_parse(), the NMLldpNeighbor is not yet added to
the NMLldpRX instance. Consequently, n->lldp_rx is NULL.

Note how we use lldp_x for logging, because we need it for the context
for which interface the logging statement is.

Thus, those debug logging statements will follow a NULL pointer and lead
to a crash.

Fixes: 630de288d2e4 ('lldp: add libnm-lldp as fork of systemd's sd_lldp_rx')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1550
(cherry picked from commit c2cddd3241349c0d5612d7603261c182fbc6d7c3)
(cherry picked from commit 8a2f7bd6e0572cc524e6bd6e4c2893e03f98a6f0)
(cherry picked from commit 6da9b98975ed790a9c00f57bd97e56c77ecb7673)
(cherry picked from commit d3434e82be1c38e8f941637a67dd4efee427432d)
---
 src/core/devices/nm-lldp-listener.c | 15 +++++++++++-
 src/libnm-lldp/nm-lldp-neighbor.c   | 47 ++++++++++++++++++++-----------------
 src/libnm-lldp/nm-lldp-neighbor.h   |  4 +++-
 src/libnm-lldp/nm-lldp-rx.c         |  2 +-
 src/libnm-lldp/nm-lldp-rx.h         |  2 +-
 5 files changed, 44 insertions(+), 26 deletions(-)

diff --git a/src/core/devices/nm-lldp-listener.c b/src/core/devices/nm-lldp-listener.c
index ac7e97f..59c8f54 100644
--- a/src/core/devices/nm-lldp-listener.c
+++ b/src/core/devices/nm-lldp-listener.c
@@ -704,9 +704,16 @@ lldp_neighbor_to_variant(LldpNeighbor *neigh)
 
 /*****************************************************************************/
 
+static void
+nmtst_lldp_event_handler(NMLldpRX *lldp, NMLldpRXEvent event, NMLldpNeighbor *n, void *user_data)
+{
+    g_assert_not_reached();
+}
+
 GVariant *
 nmtst_lldp_parse_from_raw(const guint8 *raw_data, gsize raw_len)
 {
+    nm_auto(nm_lldp_rx_unrefp) NMLldpRX             *lldp_rx     = NULL;
     nm_auto(nm_lldp_neighbor_unrefp) NMLldpNeighbor *neighbor_nm = NULL;
     nm_auto(lldp_neighbor_freep) LldpNeighbor       *neigh       = NULL;
     GVariant                                        *variant;
@@ -714,7 +721,13 @@ nmtst_lldp_parse_from_raw(const guint8 *raw_data, gsize raw_len)
     g_assert(raw_data);
     g_assert(raw_len > 0);
 
-    neighbor_nm = nm_lldp_neighbor_new_from_raw(raw_data, raw_len);
+    lldp_rx = nm_lldp_rx_new(&((NMLldpRXConfig){
+        .ifindex       = 1,
+        .neighbors_max = MAX_NEIGHBORS,
+        .callback      = nmtst_lldp_event_handler,
+    }));
+
+    neighbor_nm = nm_lldp_neighbor_new_from_raw(lldp_rx, raw_data, raw_len);
     g_assert(neighbor_nm);
 
     neigh = lldp_neighbor_new(neighbor_nm);
diff --git a/src/libnm-lldp/nm-lldp-neighbor.c b/src/libnm-lldp/nm-lldp-neighbor.c
index f1e2d42..4bb9655 100644
--- a/src/libnm-lldp/nm-lldp-neighbor.c
+++ b/src/libnm-lldp/nm-lldp-neighbor.c
@@ -65,6 +65,7 @@ parse_string(NMLldpRX *lldp_rx, char **s, const void *q, size_t n)
     const char *p = q;
     char       *k;
 
+    nm_assert(lldp_rx);
     nm_assert(s);
     nm_assert(p || n == 0);
 
@@ -99,31 +100,33 @@ parse_string(NMLldpRX *lldp_rx, char **s, const void *q, size_t n)
 }
 
 int
-nm_lldp_neighbor_parse(NMLldpNeighbor *n)
+nm_lldp_neighbor_parse(NMLldpRX *lldp_rx, NMLldpNeighbor *n)
 {
     struct ether_header h;
     const uint8_t      *p;
     size_t              left;
     int                 r;
 
+    nm_assert(lldp_rx);
     nm_assert(n);
+    nm_assert(!n->lldp_rx);
 
     if (n->raw_size < sizeof(struct ether_header)) {
-        _LOG2D(n->lldp_rx, "Received truncated packet, ignoring.");
+        _LOG2D(lldp_rx, "Received truncated packet, ignoring.");
         return -NME_UNSPEC;
     }
 
     memcpy(&h, NM_LLDP_NEIGHBOR_RAW(n), sizeof(h));
 
     if (h.ether_type != htobe16(NM_ETHERTYPE_LLDP)) {
-        _LOG2D(n->lldp_rx, "Received packet with wrong type, ignoring.");
+        _LOG2D(lldp_rx, "Received packet with wrong type, ignoring.");
         return -NME_UNSPEC;
     }
 
     if (h.ether_dhost[0] != 0x01 || h.ether_dhost[1] != 0x80 || h.ether_dhost[2] != 0xc2
         || h.ether_dhost[3] != 0x00 || h.ether_dhost[4] != 0x00
         || !NM_IN_SET(h.ether_dhost[5], 0x00, 0x03, 0x0e)) {
-        _LOG2D(n->lldp_rx, "Received packet with wrong destination address, ignoring.");
+        _LOG2D(lldp_rx, "Received packet with wrong destination address, ignoring.");
         return -NME_UNSPEC;
     }
 
@@ -138,7 +141,7 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
         uint16_t length;
 
         if (left < 2) {
-            _LOG2D(n->lldp_rx, "TLV lacks header, ignoring.");
+            _LOG2D(lldp_rx, "TLV lacks header, ignoring.");
             return -NME_UNSPEC;
         }
 
@@ -147,14 +150,14 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
         p += 2, left -= 2;
 
         if (left < length) {
-            _LOG2D(n->lldp_rx, "TLV truncated, ignoring datagram.");
+            _LOG2D(lldp_rx, "TLV truncated, ignoring datagram.");
             return -NME_UNSPEC;
         }
 
         switch (type) {
         case NM_LLDP_TYPE_END:
             if (length != 0) {
-                _LOG2D(n->lldp_rx, "End marker TLV not zero-sized, ignoring datagram.");
+                _LOG2D(lldp_rx, "End marker TLV not zero-sized, ignoring datagram.");
                 return -NME_UNSPEC;
             }
 
@@ -166,12 +169,12 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
         case NM_LLDP_TYPE_CHASSIS_ID:
             if (length < 2 || length > 256) {
                 /* includes the chassis subtype, hence one extra byte */
-                _LOG2D(n->lldp_rx, "Chassis ID field size out of range, ignoring datagram.");
+                _LOG2D(lldp_rx, "Chassis ID field size out of range, ignoring datagram.");
                 return -NME_UNSPEC;
             }
 
             if (n->id.chassis_id) {
-                _LOG2D(n->lldp_rx, "Duplicate chassis ID field, ignoring datagram.");
+                _LOG2D(lldp_rx, "Duplicate chassis ID field, ignoring datagram.");
                 return -NME_UNSPEC;
             }
 
@@ -182,12 +185,12 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
         case NM_LLDP_TYPE_PORT_ID:
             if (length < 2 || length > 256) {
                 /* includes the port subtype, hence one extra byte */
-                _LOG2D(n->lldp_rx, "Port ID field size out of range, ignoring datagram.");
+                _LOG2D(lldp_rx, "Port ID field size out of range, ignoring datagram.");
                 return -NME_UNSPEC;
             }
 
             if (n->id.port_id) {
-                _LOG2D(n->lldp_rx, "Duplicate port ID field, ignoring datagram.");
+                _LOG2D(lldp_rx, "Duplicate port ID field, ignoring datagram.");
                 return -NME_UNSPEC;
             }
 
@@ -197,12 +200,12 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
 
         case NM_LLDP_TYPE_TTL:
             if (length != 2) {
-                _LOG2D(n->lldp_rx, "TTL field has wrong size, ignoring datagram.");
+                _LOG2D(lldp_rx, "TTL field has wrong size, ignoring datagram.");
                 return -NME_UNSPEC;
             }
 
             if (n->has_ttl) {
-                _LOG2D(n->lldp_rx, "Duplicate TTL field, ignoring datagram.");
+                _LOG2D(lldp_rx, "Duplicate TTL field, ignoring datagram.");
                 return -NME_UNSPEC;
             }
 
@@ -211,26 +214,26 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
             break;
 
         case NM_LLDP_TYPE_PORT_DESCRIPTION:
-            r = parse_string(n->lldp_rx, &n->port_description, p, length);
+            r = parse_string(lldp_rx, &n->port_description, p, length);
             if (r < 0)
                 return r;
             break;
 
         case NM_LLDP_TYPE_SYSTEM_NAME:
-            r = parse_string(n->lldp_rx, &n->system_name, p, length);
+            r = parse_string(lldp_rx, &n->system_name, p, length);
             if (r < 0)
                 return r;
             break;
 
         case NM_LLDP_TYPE_SYSTEM_DESCRIPTION:
-            r = parse_string(n->lldp_rx, &n->system_description, p, length);
+            r = parse_string(lldp_rx, &n->system_description, p, length);
             if (r < 0)
                 return r;
             break;
 
         case NM_LLDP_TYPE_SYSTEM_CAPABILITIES:
             if (length != 4) {
-                _LOG2D(n->lldp_rx, "System capabilities field has wrong size.");
+                _LOG2D(lldp_rx, "System capabilities field has wrong size.");
                 return -NME_UNSPEC;
             }
 
@@ -241,13 +244,13 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
 
         case NM_LLDP_TYPE_PRIVATE:
             if (length < 4) {
-                _LOG2D(n->lldp_rx, "Found private TLV that is too short, ignoring.");
+                _LOG2D(lldp_rx, "Found private TLV that is too short, ignoring.");
                 return -NME_UNSPEC;
             }
 
             /* RFC 8520: MUD URL */
             if (memcmp(p, NM_LLDP_OUI_IANA_MUD, sizeof(NM_LLDP_OUI_IANA_MUD)) == 0) {
-                r = parse_string(n->lldp_rx,
+                r = parse_string(lldp_rx,
                                  &n->mud_url,
                                  p + sizeof(NM_LLDP_OUI_IANA_MUD),
                                  length - sizeof(NM_LLDP_OUI_IANA_MUD));
@@ -262,7 +265,7 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
 
 end_marker:
     if (!n->id.chassis_id || !n->id.port_id || !n->has_ttl) {
-        _LOG2D(n->lldp_rx, "One or more mandatory TLV missing in datagram. Ignoring.");
+        _LOG2D(lldp_rx, "One or more mandatory TLV missing in datagram. Ignoring.");
         return -NME_UNSPEC;
     }
 
@@ -739,7 +742,7 @@ nm_lldp_neighbor_new(size_t raw_size)
 }
 
 NMLldpNeighbor *
-nm_lldp_neighbor_new_from_raw(const void *raw, size_t raw_size)
+nm_lldp_neighbor_new_from_raw(NMLldpRX *lldp_rx, const void *raw, size_t raw_size)
 {
     nm_auto(nm_lldp_neighbor_unrefp) NMLldpNeighbor *n = NULL;
     int                                              r;
@@ -750,7 +753,7 @@ nm_lldp_neighbor_new_from_raw(const void *raw, size_t raw_size)
 
     nm_memcpy(NM_LLDP_NEIGHBOR_RAW(n), raw, raw_size);
 
-    r = nm_lldp_neighbor_parse(n);
+    r = nm_lldp_neighbor_parse(lldp_rx, n);
     if (r < 0)
         return NULL;
 
diff --git a/src/libnm-lldp/nm-lldp-neighbor.h b/src/libnm-lldp/nm-lldp-neighbor.h
index 1adc967..038591a 100644
--- a/src/libnm-lldp/nm-lldp-neighbor.h
+++ b/src/libnm-lldp/nm-lldp-neighbor.h
@@ -75,11 +75,13 @@ NM_LLDP_NEIGHBOR_TLV_DATA(const NMLldpNeighbor *n)
     return ((uint8_t *) NM_LLDP_NEIGHBOR_RAW(n)) + n->rindex + 2;
 }
 
+struct _NMLldpRX;
+
 int nm_lldp_neighbor_prioq_compare_func(const void *a, const void *b);
 
 void            nm_lldp_neighbor_unlink(NMLldpNeighbor *n);
 NMLldpNeighbor *nm_lldp_neighbor_new(size_t raw_size);
-int             nm_lldp_neighbor_parse(NMLldpNeighbor *n);
+int             nm_lldp_neighbor_parse(struct _NMLldpRX *lldp_rx, NMLldpNeighbor *n);
 void            nm_lldp_neighbor_start_ttl(NMLldpNeighbor *n);
 
 #endif /* __NM_LLDP_NEIGHBOR_H__ */
diff --git a/src/libnm-lldp/nm-lldp-rx.c b/src/libnm-lldp/nm-lldp-rx.c
index 6d0f4a1..551319e 100644
--- a/src/libnm-lldp/nm-lldp-rx.c
+++ b/src/libnm-lldp/nm-lldp-rx.c
@@ -255,7 +255,7 @@ lldp_rx_receive_datagram(int fd, GIOCondition condition, gpointer user_data)
     } else
         n->timestamp_usec = nm_utils_get_monotonic_timestamp_usec();
 
-    r = nm_lldp_neighbor_parse(n);
+    r = nm_lldp_neighbor_parse(lldp_rx, n);
     if (r < 0) {
         _LOG2D(lldp_rx, "Failure parsing invalid LLDP datagram.");
         return G_SOURCE_CONTINUE;
diff --git a/src/libnm-lldp/nm-lldp-rx.h b/src/libnm-lldp/nm-lldp-rx.h
index a3f3805..d96ffcd 100644
--- a/src/libnm-lldp/nm-lldp-rx.h
+++ b/src/libnm-lldp/nm-lldp-rx.h
@@ -68,7 +68,7 @@ NMLldpNeighbor **nm_lldp_rx_get_neighbors(NMLldpRX *lldp_rx, guint *out_len);
 
 /*****************************************************************************/
 
-NMLldpNeighbor *nm_lldp_neighbor_new_from_raw(const void *raw, size_t raw_size);
+NMLldpNeighbor *nm_lldp_neighbor_new_from_raw(NMLldpRX *lldp_rx, const void *raw, size_t raw_size);
 
 NMLldpNeighbor *nm_lldp_neighbor_ref(NMLldpNeighbor *n);
 NMLldpNeighbor *nm_lldp_neighbor_unref(NMLldpNeighbor *n);