[PATCH testsuite] tests/sctp: fix setting of the SCTP_EVENTS sockopt

* [PATCH testsuite] tests/sctp: fix setting of the SCTP_EVENTS sockopt
@ 2020-02-24 12:47 Ondrej Mosnacek
  2020-02-24 15:23 ` Richard Haines
  0 siblings, 1 reply; 3+ messages in thread
From: Ondrej Mosnacek @ 2020-02-24 12:47 UTC (permalink / raw)
  To: selinux

First, the setting of SCTP_EVENTS socket option in sctp_server.c is
completely wrong -- it assumes little-endian byte order and uses a plain
int instead of the dedicated sctp_event_subscribe struct.

Second, the usage in sctp_peeloff_server.c is correct, but it may lead
to errors when the SCTP header definitions are newer than what the
kernel supports. In such case the size of struct sctp_event_subscribe
may be higher than the kernel's version and the setsockopt(2) may fail
with -EINVAL due to the 'optlen > sizeof(struct sctp_event_subscribe)'
check in net/sctp/socket.c:sctp_setsockopt_events().

To fix this, introduce a common function that does what the
sctp_peeloff_server's set_subscr_events() did, but also truncates the
optlen to only those fields that we use.

Fixes: c38b57ffdac4 ("selinux-testsuite: Add SCTP test support")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 tests/sctp/sctp_common.c         | 20 ++++++++++++++++++++
 tests/sctp/sctp_common.h         |  1 +
 tests/sctp/sctp_peeloff_server.c | 28 ++++++++--------------------
 tests/sctp/sctp_server.c         |  2 +-
 4 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/tests/sctp/sctp_common.c b/tests/sctp/sctp_common.c
index 100ab22..089af2a 100644
--- a/tests/sctp/sctp_common.c
+++ b/tests/sctp/sctp_common.c
@@ -1,5 +1,8 @@
 #include "sctp_common.h"
 
+#define member_size(type, member) sizeof(((type *)0)->member)
+#define sizeof_up_to(type, member) (offsetof(type, member) + member_size(type, member))
+
 void print_context(int fd, char *text)
 {
 	char *context;
@@ -99,3 +102,20 @@ void print_ip_option(int fd, bool ipv4, char *text)
 		printf("%s No IP Options set\n", text);
 	}
 }
+
+int set_subscr_events(int fd, int data_io, int association)
+{
+	struct sctp_event_subscribe subscr_events;
+
+	memset(&subscr_events, 0, sizeof(subscr_events));
+	subscr_events.sctp_data_io_event = data_io;
+	subscr_events.sctp_association_event = association;
+
+	/*
+	 * Truncate optlen to just the fields we touch to avoid errors when
+	 * the uapi headers are newer than the running kernel.
+	 */
+	return setsockopt(fd, IPPROTO_SCTP, SCTP_EVENTS, &subscr_events,
+			  sizeof_up_to(struct sctp_event_subscribe,
+				       sctp_association_event));
+}
diff --git a/tests/sctp/sctp_common.h b/tests/sctp/sctp_common.h
index d5c1397..351ee37 100644
--- a/tests/sctp/sctp_common.h
+++ b/tests/sctp/sctp_common.h
@@ -25,3 +25,4 @@ void print_context(int fd, char *text);
 void print_addr_info(struct sockaddr *sin, char *text);
 char *get_ip_option(int fd, bool ipv4, socklen_t *opt_len);
 void print_ip_option(int fd, bool ipv4, char *text);
+int set_subscr_events(int fd, int data_io, int association);
diff --git a/tests/sctp/sctp_peeloff_server.c b/tests/sctp/sctp_peeloff_server.c
index 4a5110a..093c6c0 100644
--- a/tests/sctp/sctp_peeloff_server.c
+++ b/tests/sctp/sctp_peeloff_server.c
@@ -16,24 +16,6 @@ static void usage(char *progname)
 	exit(1);
 }
 
-static void set_subscr_events(int fd, int value)
-{
-	int result;
-	struct sctp_event_subscribe subscr_events;
-
-	memset(&subscr_events, 0, sizeof(subscr_events));
-	subscr_events.sctp_association_event = value;
-	/* subscr_events.sctp_data_io_event = value; */
-
-	result = setsockopt(fd, IPPROTO_SCTP, SCTP_EVENTS,
-			    &subscr_events, sizeof(subscr_events));
-	if (result < 0) {
-		perror("Server setsockopt: SCTP_EVENTS");
-		close(fd);
-		exit(1);
-	}
-}
-
 static sctp_assoc_t handle_event(void *buf)
 {
 	union sctp_notification *snp = buf;
@@ -166,7 +148,13 @@ int main(int argc, char **argv)
 	}
 
 	do {
-		set_subscr_events(sock, 1); /* Get assoc_id for sctp_peeloff() */
+		/* Get assoc_id for sctp_peeloff() */
+		result = set_subscr_events(sock, 0, 1);
+		if (result < 0) {
+			perror("Server setsockopt: SCTP_EVENTS");
+			close(sock);
+			exit(1);
+		}
 		sinlen = sizeof(sin);
 		flags = 0;
 
@@ -192,7 +180,7 @@ int main(int argc, char **argv)
 				exit(1);
 			}
 			/* No more notifications */
-			set_subscr_events(sock, 0);
+			set_subscr_events(sock, 0, 0);
 
 			peeloff_sk = sctp_peeloff(sock, assoc_id);
 			if (peeloff_sk < 0) {
diff --git a/tests/sctp/sctp_server.c b/tests/sctp/sctp_server.c
index 4659ed1..7f2cd20 100644
--- a/tests/sctp/sctp_server.c
+++ b/tests/sctp/sctp_server.c
@@ -134,7 +134,7 @@ int main(int argc, char **argv)
 	}
 
 	/* Enables sctp_data_io_events for sctp_recvmsg(3) for assoc_id. */
-	result = setsockopt(sock, SOL_SCTP, SCTP_EVENTS, &on, sizeof(on));
+	result = set_subscr_events(sock, on, 0);
 	if (result < 0) {
 		perror("Server setsockopt: SCTP_EVENTS");
 		close(sock);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread