All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sctp: reject forged cookie peer_addr with unknown address family
@ 2026-09-13 11:36 Xingyuan Mo
  2026-09-14 12:13 ` netdev-bot+sashiko
  0 siblings, 1 reply; 3+ messages in thread
From: Xingyuan Mo @ 2026-09-13 11:36 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner, Xin Long, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman
  Cc: open list:SCTP PROTOCOL, open list:NETWORKING [GENERAL],
	Xingyuan Mo

When cookie authentication is disabled, COOKIE-ECHO peer_addr is
attacker-controlled. An invalid sa_family made sctp_get_af_specific()
return NULL and crash in sctp_transport_init() on
af_specific->sockaddr_len. Reject non-INET/INET6 families in
sctp_unpack_cookie(), and also bail in sctp_transport_new() if
af_specific is missing.

  BUG: KASAN: null-ptr-deref in sctp_transport_new+0xa7/0x350
  Read of size 4 at addr 00000000000000b4 by task poc/682
  Call Trace:
   <IRQ>
   sctp_transport_new+0xa7/0x350
   sctp_assoc_add_peer+0x153/0x850
   sctp_process_init+0xf9/0x1180
   sctp_sf_do_5_1D_ce+0x464/0xbc0
   sctp_do_sm+0x114/0x2990
   sctp_endpoint_bh_rcv+0x280/0x430
   sctp_inq_push+0xdd/0x100
   sctp_rcv+0x17f5/0x1ae0
   sctp4_rcv+0x2b/0x40
   ip_protocol_deliver_rcu+0x25b/0x270
   ip_local_deliver+0xd1/0xe0
   </IRQ>
  Kernel panic - not syncing: Fatal exception in interrupt

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: opencode:deepseek-v4
Signed-off-by: Xingyuan Mo <hdthky0@gmail.com>
---
 net/sctp/sm_make_chunk.c | 4 ++++
 net/sctp/transport.c     | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 84a4c97d0f75..7c8fe4b38cd1 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1855,6 +1855,10 @@ struct sctp_association *sctp_unpack_cookie(
 	if (!sctp_auth_verify_cookie_params(ep, bear_cookie))
 		goto malformed;
 
+	if (bear_cookie->peer_addr.sa.sa_family != AF_INET &&
+	    bear_cookie->peer_addr.sa.sa_family != AF_INET6)
+		goto malformed;
+
 	/* Populate the association from the cookie.  */
 	memcpy(&retval->c, bear_cookie, sizeof(*bear_cookie));
 
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 6ea55b9fbde4..cd1a604d6f8e 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -92,6 +92,9 @@ struct sctp_transport *sctp_transport_new(struct net *net,
 {
 	struct sctp_transport *transport;
 
+	if (!sctp_get_af_specific(addr->sa.sa_family))
+		return NULL;
+
 	transport = kzalloc_obj(*transport, gfp);
 	if (!transport)
 		return NULL;
-- 
2.43.0


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

* Re: [PATCH] sctp: reject forged cookie peer_addr with unknown address family
  2026-09-13 11:36 [PATCH] sctp: reject forged cookie peer_addr with unknown address family Xingyuan Mo
@ 2026-09-14 12:13 ` netdev-bot+sashiko
  2026-09-14 18:45   ` Xin Long
  0 siblings, 1 reply; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-09-14 12:13 UTC (permalink / raw)
  To: hdthky0
  Cc: marcelo.leitner, lucien.xin, davem, edumazet, kuba, pabeni, horms,
	linux-sctp, netdev

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0

- [High] Incomplete fix: sctp_unpack_cookie()
  (net/sctp/sm_make_chunk.c:1858-1860) only checks that the forged…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch
  establishes the NULL-check contract for sctp_get_af_specific() at one…

--

sctp: reject forged cookie peer_addr with unknown address family

This adds a family whitelist for the COOKIE-ECHO cookie peer_addr in
sctp_unpack_cookie(), and makes sctp_transport_new() bail out when
sctp_get_af_specific() has no implementation for the requested family.
Both changes target the null dereference seen from sctp_transport_init().

> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 84a4c97d0f755..7c8fe4b38cd1c 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -1855,6 +1855,10 @@ struct sctp_association *sctp_unpack_cookie(
>  	if (!sctp_auth_verify_cookie_params(ep, bear_cookie))
>  		goto malformed;
>  
> +	if (bear_cookie->peer_addr.sa.sa_family != AF_INET &&
> +	    bear_cookie->peer_addr.sa.sa_family != AF_INET6)
> +		goto malformed;
> +

[Severity: High]
Does this check still leave the mismatched-family case open?  It only asks
whether sa_family is one of AF_INET or AF_INET6, it never relates that family
to the family of the endpoint that received the packet.

On a PF_INET listener with cookie authentication disabled the cookie contents
are attacker supplied, so sa_family can be AF_INET6.  The new guard in
sctp_transport_new() passes as well, since sctp_get_af_specific(AF_INET6) is
non-NULL on CONFIG_IPV6 kernels, and the transport ends up with
af_specific = sctp_af_inet6.

sctp_assoc_add_peer() then routes that transport using the IPv4 socket:

net/sctp/associola.c:sctp_assoc_add_peer() {
	sp = sctp_sk(asoc->base.sk);
	...
	/* Initialize the pmtu of the transport. */
	sctp_transport_route(peer, NULL, sp);
}

sctp_transport_route() calls af->get_dst(), which is sctp_v6_get_dst() here:

net/sctp/ipv6.c:sctp_v6_get_dst() {
	struct ipv6_pinfo *np = inet6_sk(sk);
	...
	final_p = fl6_update_dst(fl6, rcu_dereference(np->opt), &final);
}

inet6_sk() returns inet_sk(sk)->pinet6, which is only populated by
inet6_create(), so np is NULL for an AF_INET socket and np->opt is
dereferenced with no guard.

Isn't that the same remotely triggered null dereference in softirq context
that this patch is meant to close, only at a different offset?

For comparison, sctp_process_param() already refuses IPv6 addresses on
sockets that are not PF_INET6:

	case SCTP_PARAM_IPV6_ADDRESS:
		if (PF_INET6 != asoc->base.sk->sk_family)
			break;

Would it make more sense to validate the cookie family against the receiving
socket family, or against SCTP_INPUT_CB(skb)->af->sa_family, rather than
accepting AF_INET6 unconditionally?

>  	/* Populate the association from the cookie.  */
>  	memcpy(&retval->c, bear_cookie, sizeof(*bear_cookie));
>  
> diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> index 6ea55b9fbde45..cd1a604d6f8e3 100644
> --- a/net/sctp/transport.c
> +++ b/net/sctp/transport.c
> @@ -92,6 +92,9 @@ struct sctp_transport *sctp_transport_new(struct net *net,
>  {
>  	struct sctp_transport *transport;
>  
> +	if (!sctp_get_af_specific(addr->sa.sa_family))
> +		return NULL;
> +

[Severity: High]
This isn't a bug introduced by this patch, but the same
sctp_get_af_specific() result is used unchecked a little further along the
same COOKIE-ECHO path, in sctp_process_init():

net/sctp/sm_make_chunk.c:sctp_process_init() {
	sctp_walk_params(param, peer_init) {
		if (!src_match &&
		    (param.p->type == SCTP_PARAM_IPV4_ADDRESS ||
		     param.p->type == SCTP_PARAM_IPV6_ADDRESS)) {
			af = sctp_get_af_specific(param_type2af(param.p->type));
			if (!af->from_addr_param(&addr, param.addr,
						 chunk->sctp_hdr->source, 0))
				continue;
	...
}

The five other sctp_get_af_specific() callers in sm_make_chunk.c,
bind_addr.c and input.c all test the result for NULL first.

On a CONFIG_IPV6=n build sctp_get_af_specific(AF_INET6) is permanently NULL,
and sctp_verify_param() accepts SCTP_PARAM_IPV6_ADDRESS with a bare break, so
an attacker supplied IPv6 address parameter reaches af->from_addr_param with
af == NULL.

That block only runs when src_match is 0, and sctp_sf_do_5_1D_ce() is the one
sctp_process_init() caller that does not pass sctp_source(chunk) as peer_addr:

net/sctp/sm_statefuns.c:sctp_sf_do_5_1D_ce() {
	if (!sctp_process_init(new_asoc, chunk,
			       &chunk->subh.cookie_hdr->c.peer_addr,
			       peer_init, GFP_ATOMIC))
	...
}

So src_match stays 0 whenever the COOKIE-ECHO source address differs from the
cookie peer_addr, which is attacker chosen with cookie authentication
disabled, and also reachable with cookie authentication enabled by replaying a
legitimately obtained cookie from another source address while keeping the
same source port.

Since this is the same failure mode the patch hardens in sctp_transport_new(),
would it be worth adding the missing !af check in sctp_process_init() in the
same series?

>  	transport = kzalloc_obj(*transport, gfp);
>  	if (!transport)
>  		return NULL;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260913113522.2674588-1-hdthky0%40gmail.com

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

* Re: [PATCH] sctp: reject forged cookie peer_addr with unknown address family
  2026-09-14 12:13 ` netdev-bot+sashiko
@ 2026-09-14 18:45   ` Xin Long
  0 siblings, 0 replies; 3+ messages in thread
From: Xin Long @ 2026-09-14 18:45 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: hdthky0, marcelo.leitner, davem, edumazet, kuba, pabeni, horms,
	linux-sctp, netdev

On Mon, Sep 14, 2026 at 8:13 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0
>
> - [High] Incomplete fix: sctp_unpack_cookie()
>   (net/sctp/sm_make_chunk.c:1858-1860) only checks that the forged…
>
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but the patch
>   establishes the NULL-check contract for sctp_get_af_specific() at one…
>
> --
>
> sctp: reject forged cookie peer_addr with unknown address family
>
> This adds a family whitelist for the COOKIE-ECHO cookie peer_addr in
> sctp_unpack_cookie(), and makes sctp_transport_new() bail out when
> sctp_get_af_specific() has no implementation for the requested family.
> Both changes target the null dereference seen from sctp_transport_init().
>
> > diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> > index 84a4c97d0f755..7c8fe4b38cd1c 100644
> > --- a/net/sctp/sm_make_chunk.c
> > +++ b/net/sctp/sm_make_chunk.c
> > @@ -1855,6 +1855,10 @@ struct sctp_association *sctp_unpack_cookie(
> >       if (!sctp_auth_verify_cookie_params(ep, bear_cookie))
> >               goto malformed;
> >
> > +     if (bear_cookie->peer_addr.sa.sa_family != AF_INET &&
> > +         bear_cookie->peer_addr.sa.sa_family != AF_INET6)
> > +             goto malformed;
> > +
>
> [Severity: High]
> Does this check still leave the mismatched-family case open?  It only asks
> whether sa_family is one of AF_INET or AF_INET6, it never relates that family
> to the family of the endpoint that received the packet.
>
> On a PF_INET listener with cookie authentication disabled the cookie contents
> are attacker supplied, so sa_family can be AF_INET6.  The new guard in
> sctp_transport_new() passes as well, since sctp_get_af_specific(AF_INET6) is
> non-NULL on CONFIG_IPV6 kernels, and the transport ends up with
> af_specific = sctp_af_inet6.
>
> sctp_assoc_add_peer() then routes that transport using the IPv4 socket:
>
> net/sctp/associola.c:sctp_assoc_add_peer() {
>         sp = sctp_sk(asoc->base.sk);
>         ...
>         /* Initialize the pmtu of the transport. */
>         sctp_transport_route(peer, NULL, sp);
> }
>
> sctp_transport_route() calls af->get_dst(), which is sctp_v6_get_dst() here:
>
> net/sctp/ipv6.c:sctp_v6_get_dst() {
>         struct ipv6_pinfo *np = inet6_sk(sk);
>         ...
>         final_p = fl6_update_dst(fl6, rcu_dereference(np->opt), &final);
> }
>
> inet6_sk() returns inet_sk(sk)->pinet6, which is only populated by
> inet6_create(), so np is NULL for an AF_INET socket and np->opt is
> dereferenced with no guard.
>
> Isn't that the same remotely triggered null dereference in softirq context
> that this patch is meant to close, only at a different offset?
>
> For comparison, sctp_process_param() already refuses IPv6 addresses on
> sockets that are not PF_INET6:
>
>         case SCTP_PARAM_IPV6_ADDRESS:
>                 if (PF_INET6 != asoc->base.sk->sk_family)
>                         break;
>
> Would it make more sense to validate the cookie family against the receiving
> socket family, or against SCTP_INPUT_CB(skb)->af->sa_family, rather than
> accepting AF_INET6 unconditionally?
>
I think you can try fixing it with af->addr_valid() in
sctp_unpack_cookie(), like:

        paddr = &bear_cookie->peer_addr;
        af = sctp_get_af_specific(paddr->sa.sa_family);
        if (!af || !af->addr_valid(paddr, sctp_sk(ep->base.sk), NULL))
                goto malformed;

You may also need to add an inet.sk.sk_family check in
sctp_v6_addr_valid():

diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index ef26878f1282..12e429b3e2d1 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -734,6 +734,9 @@ static int sctp_v6_addr_valid(union sctp_addr *addr,
 {
        int ret = ipv6_addr_type(&addr->v6.sin6_addr);

+       if (sp && sp->inet.sk.sk_family != PF_INET6)
+               return 0;
+

> >       /* Populate the association from the cookie.  */
> >       memcpy(&retval->c, bear_cookie, sizeof(*bear_cookie));
> >
> > diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> > index 6ea55b9fbde45..cd1a604d6f8e3 100644
> > --- a/net/sctp/transport.c
> > +++ b/net/sctp/transport.c
> > @@ -92,6 +92,9 @@ struct sctp_transport *sctp_transport_new(struct net *net,
> >  {
> >       struct sctp_transport *transport;
> >
> > +     if (!sctp_get_af_specific(addr->sa.sa_family))
> > +             return NULL;
> > +
>
> [Severity: High]
> This isn't a bug introduced by this patch, but the same
> sctp_get_af_specific() result is used unchecked a little further along the
> same COOKIE-ECHO path, in sctp_process_init():
>
> net/sctp/sm_make_chunk.c:sctp_process_init() {
>         sctp_walk_params(param, peer_init) {
>                 if (!src_match &&
>                     (param.p->type == SCTP_PARAM_IPV4_ADDRESS ||
>                      param.p->type == SCTP_PARAM_IPV6_ADDRESS)) {
>                         af = sctp_get_af_specific(param_type2af(param.p->type));
>                         if (!af->from_addr_param(&addr, param.addr,
>                                                  chunk->sctp_hdr->source, 0))
>                                 continue;
>         ...
> }

This looks like a legit one. Please add !af to this check in
sctp_process_init().

Also, in sctp_process_param(), af->addr_valid() is needed, since we only
want unicast addresses to be added. As addr_valid() now checks sk_family
and ipv6_only_sock, we can simplify it to:

        switch (param.p->type) {
        case SCTP_PARAM_IPV6_ADDRESS:
        case SCTP_PARAM_IPV4_ADDRESS:
                af = sctp_get_af_specific(param_type2af(param.p->type));
                if (!af->from_addr_param(&addr, param.addr,
htons(asoc->peer.port), 0))
                        break;
                if (!af->addr_valid(&addr, sctp_sk(ep->base.sk), NULL))
                        break;

Thanks.

>
> The five other sctp_get_af_specific() callers in sm_make_chunk.c,
> bind_addr.c and input.c all test the result for NULL first.
>
> On a CONFIG_IPV6=n build sctp_get_af_specific(AF_INET6) is permanently NULL,
> and sctp_verify_param() accepts SCTP_PARAM_IPV6_ADDRESS with a bare break, so
> an attacker supplied IPv6 address parameter reaches af->from_addr_param with
> af == NULL.
>
> That block only runs when src_match is 0, and sctp_sf_do_5_1D_ce() is the one
> sctp_process_init() caller that does not pass sctp_source(chunk) as peer_addr:
>
> net/sctp/sm_statefuns.c:sctp_sf_do_5_1D_ce() {
>         if (!sctp_process_init(new_asoc, chunk,
>                                &chunk->subh.cookie_hdr->c.peer_addr,
>                                peer_init, GFP_ATOMIC))
>         ...
> }
>
> So src_match stays 0 whenever the COOKIE-ECHO source address differs from the
> cookie peer_addr, which is attacker chosen with cookie authentication
> disabled, and also reachable with cookie authentication enabled by replaying a
> legitimately obtained cookie from another source address while keeping the
> same source port.
>
> Since this is the same failure mode the patch hardens in sctp_transport_new(),
> would it be worth adding the missing !af check in sctp_process_init() in the
> same series?
>
> >       transport = kzalloc_obj(*transport, gfp);
> >       if (!transport)
> >               return NULL;
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260913113522.2674588-1-hdthky0%40gmail.com

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

end of thread, other threads:[~2026-09-14 18:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-13 11:36 [PATCH] sctp: reject forged cookie peer_addr with unknown address family Xingyuan Mo
2026-09-14 12:13 ` netdev-bot+sashiko
2026-09-14 18:45   ` Xin Long

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.