Change in libosmocore[master]: gprs_ns2_fr: guard against race between socket(AF_PACKET) and bind()

[laforge]

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/21666 )

Change subject: gprs_ns2_fr: guard against race between socket(AF_PACKET) and bind()
......................................................................

gprs_ns2_fr: guard against race between socket(AF_PACKET) and bind()

An AF_PACKET socket will immediately receive packets of _all_ interfaces
until it is bound to one specific interface.  This introduces a race
condition between the socket() and the bind() syscall.

Let's use the ifindex passed for each packet in recvmsg() to drop
any packets received for other interfaces.

Change-Id: I8f708ba4f9b7f76525acce17b24a8f7b125a1c1c
Related: SYS#5245
---
M src/gb/gprs_ns2_fr.c
1 file changed, 23 insertions(+), 12 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved



diff --git a/src/gb/gprs_ns2_fr.c b/src/gb/gprs_ns2_fr.c
index c774e11..0e4e2d3 100644
--- a/src/gb/gprs_ns2_fr.c
+++ b/src/gb/gprs_ns2_fr.c
@@ -86,6 +86,7 @@
 	struct osmo_fd fd;
 	char netif[IF_NAMESIZE];
 	struct osmo_fr_link *link;
+	int ifindex;
 	bool if_running;
 };
 
@@ -196,12 +197,14 @@
 	struct gprs_ns2_vc_bind *bind = bfd->data;
 	struct priv_bind *priv = bind->priv;
 	struct msgb *msg = msgb_alloc(NS_ALLOC_SIZE, "Gb/NS/FR/GRE Rx");
+	struct sockaddr_ll sll;
+	socklen_t sll_len = sizeof(sll);
 	int rc = 0;
 
 	if (!msg)
 		return -ENOMEM;
 
-	rc = read(bfd->fd, msg->data, NS_ALLOC_SIZE);
+	rc = recvfrom(bfd->fd, msg->data, NS_ALLOC_SIZE, 0, (struct sockaddr *)&sll, &sll_len);
 	if (rc < 0) {
 		LOGP(DLNS, LOGL_ERROR, "recv error %s during NS-FR-GRE recv\n",
 		     strerror(errno));
@@ -210,6 +213,11 @@
 		goto out_err;
 	}
 
+	/* ignore any packets that we might have received for a different interface, between
+	 * the socket() and the bind() call */
+	if (sll.sll_ifindex != priv->ifindex)
+		goto out_err;
+
 	msgb_put(msg, rc);
 	msg->dst = priv->link;
 	return osmo_fr_rx(msg);
@@ -299,18 +307,11 @@
 	return ifr.ifr_ifindex;
 }
 
-static int open_socket(const char *ifname)
+static int open_socket(int ifindex)
 {
 	struct sockaddr_ll addr;
-	int ifindex;
 	int fd, rc;
 
-	ifindex = devname2ifindex(ifname);
-	if (ifindex < 0) {
-		LOGP(DLNS, LOGL_ERROR, "Can not get interface index for interface %s\n", ifname);
-		return ifindex;
-	}
-
 	memset(&addr, 0, sizeof(addr));
 	addr.sll_family = AF_PACKET;
 	addr.sll_protocol = htons(ETH_P_ALL);
@@ -318,13 +319,16 @@
 
 	fd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
 	if (fd < 0) {
-		LOGP(DLNS, LOGL_ERROR, "Can not get socket for interface %s. Are you root or have CAP_RAW_SOCKET?\n", ifname);
+		LOGP(DLNS, LOGL_ERROR, "Can not create AF_PACKET socket. Are you root or have CAP_RAW_SOCKET?\n");
 		return fd;
 	}
 
+	/* there's a race condition between the above syscall and the bind() call below,
+	 * causing other packets to be received in between */
+
 	rc = bind(fd, (struct sockaddr *)&addr, sizeof(addr));
 	if (rc < 0) {
-		LOGP(DLNS, LOGL_ERROR, "Can not bind for interface %s\n", ifname);
+		LOGP(DLNS, LOGL_ERROR, "Can not bind AF_PACKET socket to ifindex %d\n", ifindex);
 		close(fd);
 		return rc;
 	}
@@ -496,7 +500,14 @@
 	fr_link->tx_cb = fr_tx_cb;
 	fr_link->tx_cb_data = bind;
 	priv->link = fr_link;
-	priv->fd.fd = rc = open_socket(netif);
+
+	priv->ifindex = devname2ifindex(netif);
+	if (priv->ifindex < 0) {
+		LOGP(DLNS, LOGL_ERROR, "Can not get interface index for interface %s\n", netif);
+		goto err_fr;
+	}
+
+	priv->fd.fd = rc = open_socket(priv->ifindex);
 	if (rc < 0)
 		goto err_fr;
 

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/21666
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I8f708ba4f9b7f76525acce17b24a8f7b125a1c1c
Gerrit-Change-Number: 21666
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201210/b65ef1da/attachment.htm>