Change in libosmo-netif[master]: src/stream: Work around more Linux kernel ABI breakage

[laforge]

laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/18628 )


Change subject: src/stream: Work around more Linux kernel ABI breakage
......................................................................

src/stream: Work around more Linux kernel ABI breakage

Back in Change-Id Ia95dd1f9ffed9f743c049e05797b1a6f1f9f8c69 we tried
to work-around kernel ABI breakage introduced in kernel >= v5.5, but it
seems that there have already been similar ABI breakages in v4.11 and v4.12.

See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/sctp.h?id=b6e6b5f1da7e8d092f86a4351802c27c0170c5a5
and https://marc.info/?l=linux-sctp&m=158729301516157&w=2 for the ost
recent incarnation.

See https://osmocom.org/issues/4573#note-6 for all known cases
of SCTP_EVENTS ABI breakage.

Change-Id: Icc49f347cdc0bb77a5c0e230597d662ac35b4acc
---
M src/stream.c
1 file changed, 71 insertions(+), 20 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/28/18628/1

diff --git a/src/stream.c b/src/stream.c
index 6e4c461..6b8f5a0 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -73,6 +73,75 @@
 #define MSG_NOSIGNAL 0
 #endif
 
+/* is any of the bytes from offset .. size in 'u8' non-zero? */
+static bool byte_nonzero(const uint8_t *u8, unsigned int offset, unsigned int u8_size)
+{
+	int j;
+
+	for (j = offset; j < u8_size; j++) {
+		if (u8[j] != 0)
+			return true;
+	}
+
+	return false;
+}
+
+/* Attempt to work around Linux kernel ABI breakage
+ *
+ * The Linux kernel ABI for the SCTP_EVENTS socket option has been broken repeatedly.
+ *  - until commit 35ea82d611da59f8bea44a37996b3b11bb1d3fd7 ( kernel < 4.11), the size is 10 bytes
+ *  - in 4.11 it is 11 bytes
+ *  - in 4.12 .. 5.4 it is 13 bytes
+ *  - in kernels >= 5.5 it is 14 bytes
+ *
+ * This wouldn't be a problem if the kernel didn't have a "stupid" assumption that the structure
+ * size passed by userspace will match 1:1 the length of the structure at kernel compile time. In
+ * an ideal world, it would just use the known first bytes and assume the remainder is all zero.
+ * But as it doesn't do that, let's try to work around this */
+static int sctp_setsockopt_events_linux_workaround(int fd, const struct sctp_event_subscribe *event)
+{
+	unsigned int size = sizeof(*event);
+	const uint8_t try_sizes[] = { 14, 13, 11, 10 };
+	uint8_t buf[14];
+	int rc, i;
+
+	/* first try to use the userspace structure as-is */
+	rc = setsockopt(fd, IPPROTO_SCTP, SCTP_EVENTS, event, sizeof(*event));
+	if (rc != -EINVAL)
+		return rc;
+
+	/* next, try with shorter sizes (older kernel + new userspace headers) */
+	for (i = 0; i < ARRAY_SIZE(try_sizes); i++) {
+		if (try_sizes[i] >= size)
+			continue;
+
+		/* ensure that if the caller requested one of the "later" options we don't
+		 * truncate */
+		if (byte_nonzero((const uint8_t *)event, try_sizes[i], size))
+			continue;
+
+		rc = setsockopt(fd, IPPROTO_SCTP, SCTP_EVENTS, event, try_sizes[i]);
+		if (rc != -EINVAL)
+			return rc;
+	}
+
+	/* finally, try with larger sizes (newer kernel + old userspace headers) */
+	if (size < sizeof(buf)) {
+		memset(buf, 0, sizeof(buf));
+		memcpy(buf, event, size);
+		for (i = 0; i < ARRAY_SIZE(try_sizes); i++) {
+			if (try_sizes[i] < size)
+				continue;
+
+			rc = setsockopt(fd, IPPROTO_SCTP, SCTP_EVENTS, buf, try_sizes[i]);
+			if (rc != -EINVAL)
+				return rc;
+		}
+	}
+
+	return rc;
+}
+
 static int sctp_sock_activate_events(int fd)
 {
 #ifdef HAVE_LIBSCTP
@@ -89,28 +158,10 @@
 	event.sctp_shutdown_event = 1;
 	/* IMPORTANT: Do NOT enable sender_dry_event here, see
 	 * https://bugzilla.redhat.com/show_bug.cgi?id=1442784 */
-	rc = setsockopt(fd, IPPROTO_SCTP, SCTP_EVENTS,
-			&event, sizeof(event));
 
-	/*
-	 * Attempt to work around kernel ABI breakage
-	 *
-	 * In kernel commit b6e6b5f1da7e8d092f86a4351802c27c0170c5a5, the
-	 * struct sctp_event_subscribe had a u8 field added to it at the end, thus
-	 * breaking ABI.
-	 * See https://marc.info/?l=linux-sctp&m=158729301516157&w=2 for discussion.
-	 *
-	 * We attempt to work around the issue where the kernel header are new
-	 * and running kernel is old, by forcing the size of the struct to 13 which
-	 * is the "old" size
-	 */
-	if ((rc < 0) && (sizeof(event) != 13))
-		rc = setsockopt(fd, IPPROTO_SCTP, SCTP_EVENTS,
-				&event, 13);
-
+	rc = sctp_setsockopt_events_linux_workaround(fd, &event);
 	if (rc < 0)
-		LOGP(DLINP, LOGL_ERROR, "couldn't activate SCTP events "
-		     "on FD %u\n", fd);
+		LOGP(DLINP, LOGL_ERROR, "couldn't activate SCTP events on FD %u\n", fd);
 	return rc;
 #else
 	return -1;

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

Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Icc49f347cdc0bb77a5c0e230597d662ac35b4acc
Gerrit-Change-Number: 18628
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200601/320d0578/attachment.htm>