Fixes Issue #157 (Router segfaults) (#159)

Found a number of problems with the router when compiling on linux

* Expected malloc()'ed memory to be zeroed resulting in segfault
* IPC_NOWAIT in iplayer loop means incoming traffic will be delayed until outgoing traffic is received.
* When configured with two BIP interfaces, broadcast loops ensue because the broacast traffic is not correctly filtered.  I think the safest thing to do here is to use the SO_BINDTODEVICE setsockopt.
This commit is contained in:
Stephen Dawson-Haggerty
2021-03-11 15:54:11 -05:00
committed by GitHub
parent 038ba9ec2b
commit adf66f412d
6 changed files with 30 additions and 19 deletions
+1 -1
View File
@@ -13,7 +13,7 @@ ifeq (${BACNET_PORT},linux)
#PFLAGS =
# -pthread
TARGET_EXT =
LIBS = -lpthread -lconfig
LIBS = -lpthread -lconfig -lm
LFLAGS += $(LIBS)
endif
+13 -3
View File
@@ -29,6 +29,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ipc.h>
#include "ipmodule.h"
#include "bacnet/bacint.h"
@@ -77,7 +78,7 @@ void *dl_ip_thread(void *pArgs)
while (!shutdown) {
/* check for incoming messages */
bacmsg = recv_from_msgbox(port->port_id, &msg_storage);
bacmsg = recv_from_msgbox(port->port_id, &msg_storage, IPC_NOWAIT);
if (bacmsg) {
switch (bacmsg->type) {
@@ -89,7 +90,7 @@ void *dl_ip_thread(void *pArgs)
&address.mac[0], &msg_data->dest.adr[0], MAX_MAC_LEN);
dl_ip_send(
&ip_data, &address, msg_data->pdu, msg_data->pdu_len);
&ip_data, &address, msg_data->pdu, msg_data->pdu_len);
check_data(msg_data);
@@ -177,8 +178,18 @@ bool dl_ip_init(ROUTER_PORT *port, IP_DATA *ip_data)
return false;
}
/* Bind to device so we don't get routing loops between our
different ports. */
status = setsockopt(ip_data->socket, SOL_SOCKET, SO_BINDTODEVICE,
port->iface, strlen(port->iface));
if (status < 0) {
close(ip_data->socket);
return false;
}
/* bind the socket to the local port number */
sin.sin_family = AF_INET;
// sin.sin_addr.s_addr, ip_data->local_addr.s_addr;// = htonl(INADDR_ANY);
sin.sin_addr.s_addr = htonl(INADDR_ANY);
sin.sin_port = ip_data->port;
@@ -307,7 +318,6 @@ int dl_ip_recv(
buff_len = 0;
PRINT(DEBUG, "BIP: src is me. Discarded!\n");
} else {
src->mac_len = 6;
memcpy(&src->mac[0], &sin.sin_addr.s_addr, 4);
+10 -11
View File
@@ -83,8 +83,6 @@ inline bool is_network_msg(BACMSG *msg);
int main(int argc, char *argv[])
{
printf("I am router\n");
ROUTER_PORT *port;
BACMSG msg_storage, *bacmsg = NULL;
MSG_DATA *msg_data = NULL;
@@ -115,7 +113,8 @@ int main(int argc, char *argv[])
}
}
bacmsg = recv_from_msgbox(head->main_id, &msg_storage);
// blocking dequeue here
bacmsg = recv_from_msgbox(head->main_id, &msg_storage, 0);
if (bacmsg) {
switch (bacmsg->type) {
case DATA: {
@@ -128,7 +127,7 @@ int main(int argc, char *argv[])
break;
}
print_msg(bacmsg);
// print_msg(bacmsg);
if (is_network_msg(bacmsg)) {
buff_len =
@@ -154,7 +153,7 @@ int main(int argc, char *argv[])
msg_storage.type = DATA;
msg_storage.data = msg_data;
print_msg(bacmsg);
// print_msg(bacmsg);
if (is_network_msg(bacmsg)) {
msg_data->ref_count = 1;
@@ -253,13 +252,13 @@ bool read_config(char *filepath)
/* create new list node to store port information */
if (head == NULL) {
head = (ROUTER_PORT *)malloc(sizeof(ROUTER_PORT));
head = (ROUTER_PORT *)calloc(sizeof(ROUTER_PORT), 1);
head->next = NULL;
current = head;
} else {
ROUTER_PORT *tmp = current;
current = current->next;
current = (ROUTER_PORT *)malloc(sizeof(ROUTER_PORT));
current = (ROUTER_PORT *)calloc(sizeof(ROUTER_PORT), 1);
current->next = NULL;
tmp->next = current;
}
@@ -273,7 +272,7 @@ bool read_config(char *filepath)
result = config_setting_lookup_string(port, "device", &iface);
if (result) {
current->iface =
(char *)malloc((strlen(iface) + 1) * sizeof(char));
(char *)calloc(sizeof(char), strlen(iface) + 1);
strcpy(current->iface, iface);
/* check if interface is valid */
@@ -315,7 +314,7 @@ bool read_config(char *filepath)
result = config_setting_lookup_string(port, "device", &iface);
if (result) {
current->iface =
(char *)malloc((strlen(iface) + 1) * sizeof(char));
(char *)calloc(sizeof(char), strlen(iface) + 1);
strcpy(current->iface, iface);
/* check if interface is valid */
@@ -455,13 +454,13 @@ bool parse_cmd(int argc, char *argv[])
/* create new list node to store port information */
if (head == NULL) {
head = (ROUTER_PORT *)malloc(sizeof(ROUTER_PORT));
head = (ROUTER_PORT *)calloc(sizeof(ROUTER_PORT), 1);
head->next = NULL;
current = head;
} else {
ROUTER_PORT *tmp = current;
current = current->next;
current = (ROUTER_PORT *)malloc(sizeof(ROUTER_PORT));
current = (ROUTER_PORT *)calloc(sizeof(ROUTER_PORT), 1);
current->next = NULL;
tmp->next = current;
}
+2 -2
View File
@@ -58,12 +58,12 @@ bool send_to_msgbox(MSGBOX_ID dest, BACMSG *msg)
return true;
}
BACMSG *recv_from_msgbox(MSGBOX_ID src, BACMSG *msg)
BACMSG *recv_from_msgbox(MSGBOX_ID src, BACMSG *msg, int flags)
{
int recv_bytes;
recv_bytes =
msgrcv(src, msg, sizeof(BACMSG) - sizeof(MSGTYPE), 0, IPC_NOWAIT);
msgrcv(src, msg, sizeof(BACMSG) - sizeof(MSGTYPE), 0, flags);
if (recv_bytes > 0) {
return msg;
} else {
+2 -1
View File
@@ -82,7 +82,8 @@ bool send_to_msgbox(
/* returns received message */
BACMSG *recv_from_msgbox(
MSGBOX_ID src,
BACMSG * msg);
BACMSG * msg,
int flags);
void del_msgbox(
MSGBOX_ID msgboxid);
+2 -1
View File
@@ -29,6 +29,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ipc.h>
#include "mstpmodule.h"
#include "bacnet/bacint.h"
#include "dlmstp_linux.h"
@@ -106,7 +107,7 @@ void *dl_mstp_thread(void *pArgs)
BACMSG msg_storage, *bacmsg;
MSG_DATA *msg_data;
bacmsg = recv_from_msgbox(port->port_id, &msg_storage);
bacmsg = recv_from_msgbox(port->port_id, &msg_storage, IPC_NOWAIT);
if (bacmsg) {
switch (bacmsg->type) {