On Tue, 2020-02-25 at 10:45 +0100, Ondrej Mosnacek wrote:
> 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@xxxxxxxxxx>
Just tested this with no problems. Thanks for the fixes.
Acked-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>
> ---
>
> v2: check the result of second set_subscr_events() call in
> sctp_peeloff_server.c
>
> tests/sctp/sctp_common.c | 20 +++++++++++++++++++
> tests/sctp/sctp_common.h | 1 +
> tests/sctp/sctp_peeloff_server.c | 33 +++++++++++++-----------------
> --
> tests/sctp/sctp_server.c | 2 +-
> 4 files changed, 35 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..8350cb4 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,12 @@ int main(int argc, char **argv)
> exit(1);
> }
> /* No more notifications */
> - set_subscr_events(sock, 0);
> + result = set_subscr_events(sock, 0, 0);
> + if (result < 0) {
> + perror("Server setsockopt:
> SCTP_EVENTS");
> + close(sock);
> + exit(1);
> + }
>
> 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);
