colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lldp: fix crash dereferencing NULL pointer during debug logging

Browse Source 
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: 630de288d2 ('lldp: add libnm-lldp as fork of systemd's sd_lldp_rx')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1550
This commit is contained in:
Thomas Haller
2024-05-31 10:34:28 +02:00
committed by Fernando Fernandez Mancera
parent 24b8d6b662
commit c2cddd3241
5 changed files with 44 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/core/devices/nm-lldp-listener.c
View File

@@ -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 * GVariant *
nmtst_lldp_parse_from_raw(const guint8 *raw_data, gsize raw_len) 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(nm_lldp_neighbor_unrefp) NMLldpNeighbor *neighbor_nm = NULL;
nm_auto(lldp_neighbor_freep) LldpNeighbor *neigh = NULL; nm_auto(lldp_neighbor_freep) LldpNeighbor *neigh = NULL;
GVariant *variant; 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_data);
g_assert(raw_len > 0); 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); g_assert(neighbor_nm);
neigh = lldp_neighbor_new(neighbor_nm); neigh = lldp_neighbor_new(neighbor_nm);

47
src/libnm-lldp/nm-lldp-neighbor.c
View File

@@ -65,6 +65,7 @@ parse_string(NMLldpRX *lldp_rx, char **s, const void *q, size_t n)
const char *p = q; const char *p = q;
char *k; char *k;
nm_assert(lldp_rx);
nm_assert(s); nm_assert(s);
nm_assert(p || n == 0); nm_assert(p || n == 0);
@@ -99,31 +100,33 @@ parse_string(NMLldpRX *lldp_rx, char **s, const void *q, size_t n)
} }
int int
nm_lldp_neighbor_parse(NMLldpNeighbor *n) nm_lldp_neighbor_parse(NMLldpRX *lldp_rx, NMLldpNeighbor *n)
{ {
struct ether_header h; struct ether_header h;
const uint8_t *p; const uint8_t *p;
size_t left; size_t left;
int r; int r;
nm_assert(lldp_rx);
nm_assert(n); nm_assert(n);
nm_assert(!n->lldp_rx);
if (n->raw_size < sizeof(struct ether_header)) { 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; return -NME_UNSPEC;
} }
memcpy(&h, NM_LLDP_NEIGHBOR_RAW(n), sizeof(h)); memcpy(&h, NM_LLDP_NEIGHBOR_RAW(n), sizeof(h));
if (h.ether_type != htobe16(NM_ETHERTYPE_LLDP)) { 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; return -NME_UNSPEC;
} }
if (h.ether_dhost[0] != 0x01 || h.ether_dhost[1] != 0x80 || h.ether_dhost[2] != 0xc2 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 || h.ether_dhost[3] != 0x00 || h.ether_dhost[4] != 0x00
|| !NM_IN_SET(h.ether_dhost[5], 0x00, 0x03, 0x0e)) { || !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; return -NME_UNSPEC;
} }
@@ -138,7 +141,7 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
uint16_t length; uint16_t length;
if (left < 2) { if (left < 2) {
_LOG2D(n->lldp_rx, "TLV lacks header, ignoring."); _LOG2D(lldp_rx, "TLV lacks header, ignoring.");
return -NME_UNSPEC; return -NME_UNSPEC;
} }
@@ -147,14 +150,14 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
p += 2, left -= 2; p += 2, left -= 2;
if (left < length) { if (left < length) {
_LOG2D(n->lldp_rx, "TLV truncated, ignoring datagram."); _LOG2D(lldp_rx, "TLV truncated, ignoring datagram.");
return -NME_UNSPEC; return -NME_UNSPEC;
} }
switch (type) { switch (type) {
case NM_LLDP_TYPE_END: case NM_LLDP_TYPE_END:
if (length != 0) { 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; return -NME_UNSPEC;
} }
@@ -166,12 +169,12 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
case NM_LLDP_TYPE_CHASSIS_ID: case NM_LLDP_TYPE_CHASSIS_ID:
if (length < 2 || length > 256) { if (length < 2 || length > 256) {
/* includes the chassis subtype, hence one extra byte */ /* 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; return -NME_UNSPEC;
} }
if (n->id.chassis_id) { 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; return -NME_UNSPEC;
} }
@@ -182,12 +185,12 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
case NM_LLDP_TYPE_PORT_ID: case NM_LLDP_TYPE_PORT_ID:
if (length < 2 || length > 256) { if (length < 2 || length > 256) {
/* includes the port subtype, hence one extra byte */ /* 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; return -NME_UNSPEC;
} }
if (n->id.port_id) { 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; return -NME_UNSPEC;
} }
@@ -197,12 +200,12 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
case NM_LLDP_TYPE_TTL: case NM_LLDP_TYPE_TTL:
if (length != 2) { 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; return -NME_UNSPEC;
} }
if (n->has_ttl) { if (n->has_ttl) {
_LOG2D(n->lldp_rx, "Duplicate TTL field, ignoring datagram."); _LOG2D(lldp_rx, "Duplicate TTL field, ignoring datagram.");
return -NME_UNSPEC; return -NME_UNSPEC;
} }
@@ -211,26 +214,26 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
break; break;
case NM_LLDP_TYPE_PORT_DESCRIPTION: 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) if (r < 0)
return r; return r;
break; break;
case NM_LLDP_TYPE_SYSTEM_NAME: 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) if (r < 0)
return r; return r;
break; break;
case NM_LLDP_TYPE_SYSTEM_DESCRIPTION: 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) if (r < 0)
return r; return r;
break; break;
case NM_LLDP_TYPE_SYSTEM_CAPABILITIES: case NM_LLDP_TYPE_SYSTEM_CAPABILITIES:
if (length != 4) { 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; return -NME_UNSPEC;
} }
@@ -241,13 +244,13 @@ nm_lldp_neighbor_parse(NMLldpNeighbor *n)
case NM_LLDP_TYPE_PRIVATE: case NM_LLDP_TYPE_PRIVATE:
if (length < 4) { 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; return -NME_UNSPEC;
} }
/* RFC 8520: MUD URL */ /* RFC 8520: MUD URL */
if (memcmp(p, NM_LLDP_OUI_IANA_MUD, sizeof(NM_LLDP_OUI_IANA_MUD)) == 0) { 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, &n->mud_url,
p + sizeof(NM_LLDP_OUI_IANA_MUD), p + sizeof(NM_LLDP_OUI_IANA_MUD),
length - 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: end_marker:
if (!n->id.chassis_id || !n->id.port_id || !n->has_ttl) { 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; return -NME_UNSPEC;
} }
@@ -741,7 +744,7 @@ nm_lldp_neighbor_new(size_t raw_size)
} }
NMLldpNeighbor * 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; nm_auto(nm_lldp_neighbor_unrefp) NMLldpNeighbor *n = NULL;
int r; int r;
@@ -752,7 +755,7 @@ nm_lldp_neighbor_new_from_raw(const void *raw, size_t raw_size)
nm_memcpy(NM_LLDP_NEIGHBOR_RAW(n), raw, 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) if (r < 0)
return NULL; return NULL;

4
src/libnm-lldp/nm-lldp-neighbor.h
View File

@@ -75,11 +75,13 @@ NM_LLDP_NEIGHBOR_TLV_DATA(const NMLldpNeighbor *n)
return ((uint8_t *) NM_LLDP_NEIGHBOR_RAW(n)) + n->rindex + 2; 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); int nm_lldp_neighbor_prioq_compare_func(const void *a, const void *b);
void nm_lldp_neighbor_unlink(NMLldpNeighbor *n); void nm_lldp_neighbor_unlink(NMLldpNeighbor *n);
NMLldpNeighbor *nm_lldp_neighbor_new(size_t raw_size); 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); void nm_lldp_neighbor_start_ttl(NMLldpNeighbor *n);
#endif /* __NM_LLDP_NEIGHBOR_H__ */ #endif /* __NM_LLDP_NEIGHBOR_H__ */

2
src/libnm-lldp/nm-lldp-rx.c
View File

@@ -255,7 +255,7 @@ lldp_rx_receive_datagram(int fd, GIOCondition condition, gpointer user_data)
} else } else
n->timestamp_usec = nm_utils_get_monotonic_timestamp_usec(); 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) { if (r < 0) {
_LOG2D(lldp_rx, "Failure parsing invalid LLDP datagram."); _LOG2D(lldp_rx, "Failure parsing invalid LLDP datagram.");
return G_SOURCE_CONTINUE; return G_SOURCE_CONTINUE;

2
src/libnm-lldp/nm-lldp-rx.h
View File

@@ -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_ref(NMLldpNeighbor *n);
NMLldpNeighbor *nm_lldp_neighbor_unref(NMLldpNeighbor *n); NMLldpNeighbor *nm_lldp_neighbor_unref(NMLldpNeighbor *n);