sctp_assoc_update() folds a temporary association into the existing one
when an INIT collision is resolved. It copies asoc->c, peer.rwnd,
peer.sack_needed, peer.auth_capable and peer.i, and nothing else.

The remaining peer capability bits therefore keep whatever the surviving
association was given when it was created, rather than what the peer
advertised in the INIT that caused the collision.

Forward TSN is the visible case. The INIT-ACK is built from the
temporary association, so it advertises Forward-TSN-Supported; once the
collision is resolved the surviving association holds
peer.prsctp_capable == 0, and the first FORWARD TSN chunk the peer sends
is answered with ERROR "Unrecognized chunk type". The peer does not
expect this, having been told the capability was supported. ecn_capable,
asconf_capable, reconf_capable and intl_capable are lost in the same
way.

The two address flags fail the other way round. sctp_process_param()
clears ipv4_address and ipv6_address and sets them from the peer's
Supported Address Types, but only on the temporary association. The
surviving association keeps the permissive defaults from
sctp_association_init(), so it can believe a peer supports an address
family that peer never advertised.

peer.auth_capable is already carried, added by commit 1be9a950c646
("net: sctp: inherit auth_capable on INIT collisions") for the same
reason. This extends that to the rest of the block.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Warren Briggs <wbriggs@cellusys.com>
---
Changes since v2:
 - dropped asoc->peer.hostname_address. The field no longer exists,
   removed by commit bd4b28189469 ("sctp: delete the obsolete code for
   the host name address param").
 - wrapped the commit message at 75 columns.
 - added the Fixes tag.
 - retargeted at net, subject prefix corrected.

Changes since v1:
 - added ecn_capable, asconf_capable, reconf_capable and intl_capable,
   the missing fields identified in review of v1.
 - also added ipv4_address and ipv6_address, which are set from the
   peer's Supported Address Types on the temporary association and are
   lost at the merge in the same way.

Testing. An INIT collision was resolved between two sockets on one host
and the resulting association's peer capabilities read back, with a
second, non-collided association created in the same run as a control,
on an unpatched and a patched kernel:

prsctp_capable   SCTP_PR_SUPPORTED            unpatched 0, patched 1
reconf_capable   SCTP_RECONFIG_SUPPORTED      unpatched 0, patched 1
intl_capable     SCTP_INTERLEAVING_SUPPORTED  unpatched 0, patched 1
asconf_capable   SCTP_ASCONF_SUPPORTED        unpatched 0, patched 1
ecn_capable      SCTP_ECN_SUPPORTED           unpatched 0, patched 1
ipv4_address     sctp_diag sctpi_peer_capable unpatched 1, patched 0,
                 with the peer advertising IPv6 only
ipv6_address     sctp_diag sctpi_peer_capable unpatched 1, patched 0,
                 with the peer advertising IPv4 only

Three of those were also confirmed on the wire. An unpatched kernel that
has advertised Forward-TSN-Supported in its INIT-ACK answers a FORWARD
TSN chunk with ERROR cause 6; a patched one accepts it.
SCTP_RESET_STREAMS and sctp_bindx(SCTP_BINDX_ADD_ADDR) put a RE-CONFIG
and an ASCONF on the wire on a patched kernel and produce nothing on an
unpatched one.

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 4521be3..0bd0a66 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1107,6 +1107,13 @@ int sctp_assoc_update(struct sctp_association *asoc,
 	asoc->peer.rwnd = new->peer.rwnd;
 	asoc->peer.sack_needed = new->peer.sack_needed;
 	asoc->peer.auth_capable = new->peer.auth_capable;
+	asoc->peer.prsctp_capable = new->peer.prsctp_capable;
+	asoc->peer.ecn_capable = new->peer.ecn_capable;
+	asoc->peer.asconf_capable = new->peer.asconf_capable;
+	asoc->peer.reconf_capable = new->peer.reconf_capable;
+	asoc->peer.intl_capable = new->peer.intl_capable;
+	asoc->peer.ipv4_address = new->peer.ipv4_address;
+	asoc->peer.ipv6_address = new->peer.ipv6_address;
 	asoc->peer.i = new->peer.i;
 
 	if (!sctp_tsnmap_init(&asoc->peer.tsn_map, SCTP_TSN_MAP_INITIAL,

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] sctp_assoc_update() (net/sctp/associola.c:1114) now copies
  new->peer.intl_capable into the surviving association but never re-…
- [High] sctp_assoc_update() now enables peer.asconf_capable
  (net/sctp/associola.c:1112) and peer.reconf_capable (associola.c:1113)…

Pre-existing issues:
- [Medium] The collision merge still does not carry
  asoc->peer.adaptation_ind.

--

sctp: carry peer capabilities across an INIT collision

sctp_assoc_update() folds a temporary association into an existing one when
an INIT collision is resolved, and today it only copies asoc->c, peer.rwnd,
peer.sack_needed, peer.auth_capable and peer.i.  This patch extends the copy
block with prsctp_capable, ecn_capable, asconf_capable, reconf_capable,
intl_capable, ipv4_address and ipv6_address so the surviving association
reflects what the peer advertised in the colliding INIT.

A few questions about the state that is derived from those bits but is not
refreshed by the merge.

> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 4521be3bd85aa..0bd0a662b5463 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1107,6 +1107,13 @@ int sctp_assoc_update(struct sctp_association *asoc,
>  	asoc->peer.rwnd = new->peer.rwnd;
>  	asoc->peer.sack_needed = new->peer.sack_needed;
>  	asoc->peer.auth_capable = new->peer.auth_capable;
> +	asoc->peer.prsctp_capable = new->peer.prsctp_capable;
> +	asoc->peer.ecn_capable = new->peer.ecn_capable;
> +	asoc->peer.asconf_capable = new->peer.asconf_capable;
> +	asoc->peer.reconf_capable = new->peer.reconf_capable;

[Severity: High]

Turning these two bits on lets the surviving association accept inbound
ASCONF and RE-CONFIG, but the receive-side sequence counters those features
compare against are still the survivor's own.

Both are derived from the peer's Initial TSN in sctp_process_init(), which in
a collision runs on the temporary association only:

net/sctp/sm_make_chunk.c:sctp_process_init() {
	...
	asoc->strreset_inseq = asoc->peer.i.initial_tsn;
	...
	asoc->peer.addip_serial = asoc->peer.i.initial_tsn - 1;
}

sctp_assoc_update() copies asoc->peer.i but leaves peer.addip_serial and
strreset_inseq untouched, so a COOKIE_WAIT survivor keeps zero for both.

Can the peer's ASCONF then never be processed?  sctp_sf_do_asconf() now
clears the capability gate and reaches:

net/sctp/sm_statefuns.c:sctp_sf_do_asconf() {
	...
	if (serial == asoc->peer.addip_serial + 1) {
	...
	} else if (serial < asoc->peer.addip_serial + 1) {
	...
	} else {
		/* discarded */
	}
}

With the peer's serial equal to its Initial TSN and peer.addip_serial at 0,
neither arm matches and the chunk lands in the final else, so no ASCONF-ACK
is sent.  Does the peer then retransmit until its T4 limit and give up on
ASCONF entirely?

The RE-CONFIG side has the same shape.  sctp_process_strreset_outreq(), and
equally the inreq/tsnreq/addstrm_out/addstrm_in handlers, validate against
the stale strreset_inseq:

net/sctp/stream.c:sctp_process_strreset_outreq() {
	...
	if (TSN_lt(asoc->strreset_inseq, request_seq) ||
	    TSN_lt(request_seq, asoc->strreset_inseq - 2)) {
		result = SCTP_STRRESET_ERR_BAD_SEQNO;
		goto err;
	}
}

The testing notes describe ASCONF and RE-CONFIG appearing on the wire, which
exercises the outbound counters (addip_serial and strreset_outseq, both set
locally from asoc->c.initial_tsn).  Was the inbound direction tested too?

Should the merge also carry strreset_inseq and peer.addip_serial, or
re-derive them from new->peer.i.initial_tsn the way sctp_process_init() does?

> +	asoc->peer.intl_capable = new->peer.intl_capable;

[Severity: High]

This bit selects the chunk operation set, and the merge copies the bit
without re-selecting it.  The only place asoc->stream.si is derived from it
is sctp_stream_interleave_init(), reached from sctp_stream_init():

net/sctp/stream_interleave.c:sctp_stream_interleave_init() {
	...
	stream->si = asoc->peer.intl_capable ? &sctp_stream_interleave_1
					     : &sctp_stream_interleave_0;
}

In a collision that ran on the temporary association.  Neither branch of
sctp_assoc_update() refreshes it on the survivor: sctp_stream_update() copies
out/in/outcnt/incnt and leaves stream->si alone, sctp_stream_clear() only
zeroes mid/mid_uo, and the COOKIE_ECHOED case calls neither.  asoc->frag_point
is not recomputed either, although normal negotiation pairs the two:

net/sctp/sm_make_chunk.c:sctp_process_init() {
	...
	if (sctp_stream_init(&asoc->stream, ...))
		goto clean_up;

	/* Update frag_point when stream_interleave may get changed. */
	sctp_assoc_update_frag_point(asoc);
}

So after the merge peer.intl_capable is 1 while stream.si is still
&sctp_stream_interleave_0.  The INIT-ACK extension list is built from
peer.intl_capable, so the peer is told I-DATA is in use.  What happens when
it sends one?

net/sctp/sm_statefuns.c:sctp_eat_data() {
	...
	if (!asoc->stream.si->validate_data(chunk))
		return SCTP_IERROR_PROTO_VIOLATION;
}

sctp_validate_data() returns false for anything that is not SCTP_CID_DATA,
and the callers turn SCTP_IERROR_PROTO_VIOLATION into
sctp_sf_abort_violation().  Can a peer that legitimately follows the
negotiated extension abort the association this way?

There is a second effect in the send path, which branches directly on the
bit rather than on stream.si:

net/sctp/stream_sched.c:sctp_sched_dequeue_done() {
	...
	if (!list_is_last(&ch->frag_list, &ch->msg->chunks) &&
	    !q->asoc->peer.intl_capable) {
		/* pin stream.out_curr */
	}
}

With the bit set but plain DATA chunks being dequeued, stream.out_curr is no
longer pinned mid-message.  Can the PRIO/RR schedulers then interleave
fragments of different streams on the wire?  Before this patch the survivor
kept intl_capable at 0 and the pin was taken.

The reverse case looks reachable too: a restart where the new INIT no longer
advertises I-DATA now clears the bit while stream.si stays
&sctp_stream_interleave_1, leaving I-DATA and I-FORWARD-TSN installed for a
peer that did not negotiate them.

Would calling sctp_stream_interleave_init() and sctp_assoc_update_frag_point()
on the surviving association after the copy address both directions?

> +	asoc->peer.ipv4_address = new->peer.ipv4_address;
> +	asoc->peer.ipv6_address = new->peer.ipv6_address;
>  	asoc->peer.i = new->peer.i;

[Severity: Medium]

This is a pre-existing issue and not introduced by this patch, but since the
copy block is being extended: peer.adaptation_ind is still not carried.

sctp_process_param() records the peer's Adaptation Layer Indication on the
association it is processing, which in a collision is the temporary one:

net/sctp/sm_make_chunk.c:sctp_process_param() {
	...
	case SCTP_PARAM_ADAPTATION_LAYER_IND:
		asoc->peer.adaptation_ind = ntohl(param.aind->adaptation_ind);
}

and the completion path reads the survivor's copy right after the merge:

net/sctp/sm_statefuns.c:sctp_sf_do_dupcook_b() {
	...
	if (asoc->peer.adaptation_ind)
		sctp_add_cmd_sf(commands, SCTP_CMD_ADAPTATION_IND, SCTP_NULL());
}

For a COOKIE_WAIT survivor the field is still 0, so is the
SCTP_ADAPTATION_INDICATION notification dropped even though the peer asked
for it, and on a restart is an older non-zero value reported instead of the
new one?  asoc->c = new->c does update c.adaptation_ind, but nothing copies
that into peer.adaptation_ind after the merge.

Note this field is a separate __u32 rather than part of the bitfield block
the changelog calls "the rest of the block", so it is outside what the patch
sets out to fix.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921135733.577647-1-wbriggs%40cellusys.com