Skip to content

Commit 88fe3fa

Browse files
vincentbernatvincentbernat
committed
daemon/netlink: use a different socket for changes and queries
There is a race condition when using the same socket for both. We need to subscribe for changes before getting the current state as we don't want to miss an update happening while we get the initial state, but if there is such an update, the Netlink messages we receive may not be the ones we expect. Fix #611
1 parent f2d5c6f commit 88fe3fa

File tree

2 files changed

+54
-33
lines changed

2 files changed

+54
-33
lines changed

‎NEWS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ lldpd (1.0.18)
44
* Fix:
55
+ Fix memory leaks in EDP/FDP decoding when receiving some TLVs twice.
66
+ Do not set interface description continuously.
7+
+ Use a different Netlink socket for changes and queries.
78

89
lldpd (1.0.17)
910
* Fix:

‎src/daemon/netlink.c‎

Lines changed: 53 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ struct netlink_req {
3838
};
3939

4040
struct lldpd_netlink {
41-
int nl_socket;
41+
int nl_socket_queries;
42+
int nl_socket_changes;
4243
int nl_socket_recv_size;
4344
/* Cache */
4445
struct interfaces_device_list *devices;
@@ -94,48 +95,60 @@ netlink_socket_set_buffer_size(int s, int optname, const char *optname_str, int
9495
* @return 0 on success, -1 otherwise
9596
*/
9697
static int
97-
netlink_connect(struct lldpd *cfg, int protocol, unsigned groups)
98+
netlink_connect(struct lldpd *cfg, unsigned groups)
9899
{
99-
int s;
100+
int s1 = -1, s2 = -1;
100101
struct sockaddr_nl local = { .nl_family = AF_NETLINK,
101102
.nl_pid = 0,
102103
.nl_groups = groups };
103104

104-
/* Open Netlink socket */
105-
log_debug("netlink", "opening netlink socket");
106-
s = socket(AF_NETLINK, SOCK_RAW, protocol);
107-
if (s == -1) {
108-
log_warn("netlink", "unable to open netlink socket");
109-
return -1;
105+
/* Open Netlink socket for subscriptions */
106+
log_debug("netlink", "opening netlink sockets");
107+
s1 = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
108+
if (s1 == -1) {
109+
log_warn("netlink", "unable to open netlink socket for changes");
110+
goto error;
110111
}
111112
if (NETLINK_SEND_BUFSIZE &&
112-
netlink_socket_set_buffer_size(s, SO_SNDBUF, "SO_SNDBUF",
113+
netlink_socket_set_buffer_size(s1, SO_SNDBUF, "SO_SNDBUF",
113114
NETLINK_SEND_BUFSIZE) == -1) {
114-
close(s);
115-
return -1;
115+
log_warn("netlink", "unable to set send buffer size");
116+
goto error;
116117
}
117118

118-
int rc = netlink_socket_set_buffer_size(s, SO_RCVBUF, "SO_RCVBUF",
119+
int rc = netlink_socket_set_buffer_size(s1, SO_RCVBUF, "SO_RCVBUF",
119120
NETLINK_RECEIVE_BUFSIZE);
120121
switch (rc) {
121122
case -1:
122-
close(s);
123-
return -1;
123+
log_warn("netlink", "unable to set receiver buffer size");
124+
goto error;
124125
case -2:
126+
/* Cannot set size */
125127
cfg->g_netlink->nl_socket_recv_size = 0;
126128
break;
127129
default:
128130
cfg->g_netlink->nl_socket_recv_size = rc;
129131
break;
130132
}
131133
if (groups &&
132-
bind(s, (struct sockaddr *)&local, sizeof(struct sockaddr_nl)) < 0) {
134+
bind(s1, (struct sockaddr *)&local, sizeof(struct sockaddr_nl)) < 0) {
133135
log_warn("netlink", "unable to bind netlink socket");
134-
close(s);
135-
return -1;
136+
goto error;
136137
}
137-
cfg->g_netlink->nl_socket = s;
138+
139+
/* Opening Netlink socket to for queries */
140+
s2 = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
141+
if (s2 == -1) {
142+
log_warn("netlink", "unable to open netlink socket for queries");
143+
goto error;
144+
}
145+
cfg->g_netlink->nl_socket_changes = s1;
146+
cfg->g_netlink->nl_socket_queries = s2;
138147
return 0;
148+
error:
149+
if (s1 != -1) close(s1);
150+
if (s2 != -1) close(s2);
151+
return -1;
139152
}
140153

141154
/**
@@ -525,13 +538,12 @@ netlink_merge(struct interfaces_device *old, struct interfaces_device *new)
525538
* @return 0 on success, -1 on error
526539
*/
527540
static int
528-
netlink_recv(struct lldpd *cfg, struct interfaces_device_list *ifs,
541+
netlink_recv(struct lldpd *cfg, int s, struct interfaces_device_list *ifs,
529542
struct interfaces_address_list *ifas)
530543
{
531544
int end = 0, ret = 0, flags, retry = 0;
532545
struct iovec iov;
533546
int link_update = 0;
534-
int s = cfg->g_netlink->nl_socket;
535547

536548
struct interfaces_device *ifdold;
537549
struct interfaces_device *ifdnew;
@@ -570,8 +582,10 @@ netlink_recv(struct lldpd *cfg, struct interfaces_device_list *ifs,
570582
}
571583
int rsize = cfg->g_netlink->nl_socket_recv_size;
572584
if (errno == ENOBUFS && rsize > 0 &&
573-
rsize < NETLINK_MAX_RECEIVE_BUFSIZE) {
574-
/* Try to increase buffer size */
585+
rsize < NETLINK_MAX_RECEIVE_BUFSIZE &&
586+
s == cfg->g_netlink->nl_socket_changes) {
587+
/* Try to increase buffer size, only for the
588+
* socket used to receive changes */
575589
rsize *= 2;
576590
if (rsize > NETLINK_MAX_RECEIVE_BUFSIZE) {
577591
rsize = NETLINK_MAX_RECEIVE_BUFSIZE;
@@ -843,7 +857,7 @@ netlink_subscribe_changes(struct lldpd *cfg)
843857
netlink_group_mask(RTNLGRP_IPV4_IFADDR) |
844858
netlink_group_mask(RTNLGRP_IPV6_IFADDR);
845859

846-
return netlink_connect(cfg, NETLINK_ROUTE, groups);
860+
return netlink_connect(cfg, groups);
847861
}
848862

849863
/**
@@ -852,7 +866,8 @@ static void
852866
netlink_change_cb(struct lldpd *cfg)
853867
{
854868
if (cfg->g_netlink == NULL) return;
855-
netlink_recv(cfg, cfg->g_netlink->devices, cfg->g_netlink->addresses);
869+
netlink_recv(cfg, cfg->g_netlink->nl_socket_changes, cfg->g_netlink->devices,
870+
cfg->g_netlink->addresses);
856871
}
857872

858873
/**
@@ -897,30 +912,32 @@ netlink_initialize(struct lldpd *cfg)
897912
}
898913
TAILQ_INIT(ifs);
899914

900-
if (netlink_send(cfg->g_netlink->nl_socket, RTM_GETADDR, AF_UNSPEC, 1) == -1)
915+
if (netlink_send(cfg->g_netlink->nl_socket_queries, RTM_GETADDR, AF_UNSPEC,
916+
1) == -1)
901917
goto end;
902-
netlink_recv(cfg, NULL, ifaddrs);
903-
if (netlink_send(cfg->g_netlink->nl_socket, RTM_GETLINK, AF_PACKET, 2) == -1)
918+
netlink_recv(cfg, cfg->g_netlink->nl_socket_queries, NULL, ifaddrs);
919+
if (netlink_send(cfg->g_netlink->nl_socket_queries, RTM_GETLINK, AF_PACKET,
920+
2) == -1)
904921
goto end;
905-
netlink_recv(cfg, ifs, NULL);
922+
netlink_recv(cfg, cfg->g_netlink->nl_socket_queries, ifs, NULL);
906923
#ifdef ENABLE_DOT1
907924
/* If we have a bridge, search for VLAN-aware bridges */
908925
TAILQ_FOREACH (iff, ifs, next) {
909926
if (iff->type & IFACE_BRIDGE_T) {
910927
log_debug("netlink",
911928
"interface %s is a bridge, check for VLANs", iff->name);
912-
if (netlink_send(cfg->g_netlink->nl_socket, RTM_GETLINK,
929+
if (netlink_send(cfg->g_netlink->nl_socket_queries, RTM_GETLINK,
913930
AF_BRIDGE, 3) == -1)
914931
goto end;
915-
netlink_recv(cfg, ifs, NULL);
932+
netlink_recv(cfg, cfg->g_netlink->nl_socket_queries, ifs, NULL);
916933
break;
917934
}
918935
}
919936
#endif
920937

921938
/* Listen to any future change */
922939
cfg->g_iface_cb = netlink_change_cb;
923-
if (levent_iface_subscribe(cfg, cfg->g_netlink->nl_socket) == -1) {
940+
if (levent_iface_subscribe(cfg, cfg->g_netlink->nl_socket_changes) == -1) {
924941
goto end;
925942
}
926943

@@ -937,7 +954,10 @@ void
937954
netlink_cleanup(struct lldpd *cfg)
938955
{
939956
if (cfg->g_netlink == NULL) return;
940-
if (cfg->g_netlink->nl_socket != -1) close(cfg->g_netlink->nl_socket);
957+
if (cfg->g_netlink->nl_socket_changes != -1)
958+
close(cfg->g_netlink->nl_socket_changes);
959+
if (cfg->g_netlink->nl_socket_queries != -1)
960+
close(cfg->g_netlink->nl_socket_queries);
941961
interfaces_free_devices(cfg->g_netlink->devices);
942962
interfaces_free_addresses(cfg->g_netlink->addresses);
943963

0 commit comments

Comments
 (0)