Re: [PATCH net] sctp: validate Cookie Preservative parameter length

On Wed, Jul 29, 2026 at 10:54=E2=80=AFAM Charles Vosburgh via B4 Relay
<[email protected]> wrote:
>
> From: Charles Vosburgh <[email protected]>
>
> The Cookie Preservative parameter contains a fixed 32-bit Suggested
> Cookie Life-span Increment after its parameter header. However,
> sctp_verify_param() accepts a header-only parameter because the generic
> parameter walker only requires the header to be present.
>
> sctp_process_param() then reads lifespan_increment beyond the declared
> parameter and adds the value to asoc->cookie_life. When the malformed
> parameter is last in an INIT, the read starts at the receive skb tail,
> and the resulting value changes the expiration in the State Cookie
> returned in the INIT ACK.
>
> Require the declared parameter length to match the fixed structure size
> and abort the association through the existing invalid parameter length
> path otherwise.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Suggested-by: Xin Long <[email protected]>
> Cc: [email protected]
> Signed-off-by: Charles Vosburgh <[email protected]>
> ---
> This follows Xin Long's review of "[PATCH net] sctp: validate Adaptation
> Indication parameter length", which noted that
> SCTP_PARAM_COOKIE_PRESERVATIVE has the same fixed-format validation issue=
.
>
> Runtime-tested in KVM/QEMU. The unchanged reproducer accepts header-only
> and overlong Cookie Preservative parameters on the unpatched kernel. With
> this patch applied to net.git base 51b093a7ba27, both malformed forms are
> rejected while the baseline and valid eight-byte controls still pass.
>
> The patch passes checkpatch --strict, builds net/sctp/sm_make_chunk.o,
> and completes a full x86-64 bzImage build.
> ---
>  net/sctp/sm_make_chunk.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index a1c0334a1038..eb9cff77759b 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -2165,7 +2165,14 @@ static enum sctp_ierror sctp_verify_param(struct n=
et *net,
>         switch (param.p->type) {
>         case SCTP_PARAM_IPV4_ADDRESS:
>         case SCTP_PARAM_IPV6_ADDRESS:
> +               break;
>         case SCTP_PARAM_COOKIE_PRESERVATIVE:
> +               if (ntohs(param.p->length) !=3D sizeof(*param.life)) {
> +                       sctp_process_inv_paramlength(asoc, param.p,
> +                                                    chunk, err_chunk);
> +                       retval =3D SCTP_IERROR_ABORT;
> +               }
> +               break;
Why not move 'case SCTP_PARAM_COOKIE_PRESERVATIVE' down
to after 'case SCTP_PARAM_ECN_CAPABLE'? it will save a 'break;'.

Thanks.

>         case SCTP_PARAM_SUPPORTED_ADDRESS_TYPES:
>         case SCTP_PARAM_STATE_COOKIE:
>         case SCTP_PARAM_HEARTBEAT_INFO:
>
> ---
> base-commit: 51b093a7ba27476e1f639455f005e8d2e75390e4
> change-id: 20260729-sctp-cookie-preservative-length-ffa4472272fb
>
> Best regards,
> --
> Charles Vosburgh <[email protected]>
>
>