Re: [LTP] [PATCH] cve/sctphantom: fix EINVAL on pre-v4.19 kernels

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

> The test passed its 156 byte mirror of struct sctp_paddrparams, which
> includes the spp_ipv6_flowlabel and spp_dscp tail, to
> SCTP_PEER_ADDR_PARAMS. Kernels older than v4.19 only accept
> optlen == sizeof(struct sctp_paddrparams), which is 152 there, so the
> setsockopt() failed and the test broke:

>     sctphantom.c:361: TBROK: setsockopt(6, 132, 9, 0x..., 156) failed: EINVAL (22)

> Drop the tail fields from the mirrored struct so that the option length
> matches the pre-v4.19 layout. That length is also accepted by v4.19 and
> newer kernels as the compat size introduced along with the tail fields
> by upstream commit 0b0dce7a36fb ("sctp: add spp_ipv6_flowlabel and
> spp_dscp for sctp_paddrparams"), provided the SPP_DSCP and
> SPP_IPV6_FLOWLABEL flags are not set, which the test never does.

Indeed, it was added in 0b0dce7a36fb ("sctp: add spp_ipv6_flowlabel and spp_dscp
for sctp_paddrparams") in v4.19-rc1. Thanks!

Reviewed-by: Petr Vorel <pvorel@suse.cz>

> -/* Mirror of the uapi struct sctp_paddrparams */
> +/*
> + * Mirror of the uapi struct sctp_paddrparams without the spp_ipv6_flowlabel
> + * and spp_dscp tail added in v4.19: older kernels require optlen == 152,
> + * while v4.19 and newer also accept the pre-v4.19 length.
> + */
>  struct tst_sctp_paddrparams {
> -	int32_t		spp_assoc_id;
> +	int32_t			spp_assoc_id;
>  	struct sockaddr_storage	spp_address;
> -	uint32_t	spp_hbinterval;
> -	uint16_t	spp_pathmaxrxt;
> -	uint32_t	spp_pathmtu;
> -	uint32_t	spp_sackdelay;
> -	uint32_t	spp_flags;
> -	uint32_t	spp_ipv6_flowlabel;
> -	uint8_t		spp_dscp;
> +	uint32_t		spp_hbinterval;
> +	uint16_t		spp_pathmaxrxt;
> +	uint32_t		spp_pathmtu;
> +	uint32_t		spp_sackdelay;
> +	uint32_t		spp_flags;
>  } __attribute__((packed, aligned(4)));

nit: I suppose you copy paste the older struct, which used a different
whitespace. But how about (before merge) just remove spp_ipv6_flowlabel and
spp_dscp? Without touching whitespace of the other members is change more
visible even without using --ignore-space-change.

Kind regards,
Petr

+++ testcases/cve/sctphantom.c
@@ -62,7 +62,11 @@
 #define ADDR_DUMMY	"127.0.0.254"	/* third path to keep transport_count > 1 */
 #define ADDR_SPOOF	"127.0.0.3"	/* forged ASCONF packet source */
 
-/* Mirror of the uapi struct sctp_paddrparams */
+/*
+ * Mirror of the uapi struct sctp_paddrparams without the spp_ipv6_flowlabel
+ * and spp_dscp tail added in v4.19: older kernels require optlen == 152,
+ * while v4.19 and newer also accept the pre-v4.19 length.
+ */
 struct tst_sctp_paddrparams {
 	int32_t			spp_assoc_id;
 	struct sockaddr_storage	spp_address;
@@ -71,8 +75,6 @@ struct tst_sctp_paddrparams {
 	uint32_t		spp_pathmtu;
 	uint32_t		spp_sackdelay;
 	uint32_t		spp_flags;
-	uint32_t	spp_ipv6_flowlabel;
-	uint8_t		spp_dscp;
 } __attribute__((packed, aligned(4)));
 
 /* Mirror of the uapi struct sctp_paddrinfo */

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp