Skip to content

Commit 35b4f24

Browse files
lxindavem330
authored andcommitted
sctp: do asoc update earlier in sctp_sf_do_dupcook_a
There's a panic that occurs in a few of envs, the call trace is as below: [] general protection fault, ... 0x29acd70f1000a: 0000 [#1] SMP PTI [] RIP: 0010:sctp_ulpevent_notify_peer_addr_change+0x4b/0x1fa [sctp] [] sctp_assoc_control_transport+0x1b9/0x210 [sctp] [] sctp_do_8_2_transport_strike.isra.16+0x15c/0x220 [sctp] [] sctp_cmd_interpreter.isra.21+0x1231/0x1a10 [sctp] [] sctp_do_sm+0xc3/0x2a0 [sctp] [] sctp_generate_timeout_event+0x81/0xf0 [sctp] This is caused by a transport use-after-free issue. When processing a duplicate COOKIE-ECHO chunk in sctp_sf_do_dupcook_a(), both COOKIE-ACK and SHUTDOWN chunks are allocated with the transort from the new asoc. However, later in the sideeffect machine, the old asoc is used to send them out and old asoc's shutdown_last_sent_to is set to the transport that SHUTDOWN chunk attached to in sctp_cmd_setup_t2(), which actually belongs to the new asoc. After the new_asoc is freed and the old asoc T2 timeout, the old asoc's shutdown_last_sent_to that is already freed would be accessed in sctp_sf_t2_timer_expire(). Thanks Alexander and Jere for helping dig into this issue. To fix it, this patch is to do the asoc update first, then allocate the COOKIE-ACK and SHUTDOWN chunks with the 'updated' old asoc. This would make more sense, as a chunk from an asoc shouldn't be sent out with another asoc. We had fixed quite a few issues caused by this. Fixes: 145cb2f ("sctp: Fix bundling of SHUTDOWN with COOKIE-ACK") Reported-by: Alexander Sverdlin <[email protected]> Reported-by: [email protected] Reported-by: Michal Tesar <[email protected]> Signed-off-by: Xin Long <[email protected]> Acked-by: Marcelo Ricardo Leitner <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent c5197b4 commit 35b4f24

File tree

1 file changed

+20
-5
lines changed

1 file changed

+20
-5
lines changed

net/sctp/sm_statefuns.c

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1852,20 +1852,35 @@ static enum sctp_disposition sctp_sf_do_dupcook_a(
18521852
SCTP_TO(SCTP_EVENT_TIMEOUT_T4_RTO));
18531853
sctp_add_cmd_sf(commands, SCTP_CMD_PURGE_ASCONF_QUEUE, SCTP_NULL());
18541854

1855-
repl = sctp_make_cookie_ack(new_asoc, chunk);
1855+
/* Update the content of current association. */
1856+
if (sctp_assoc_update((struct sctp_association *)asoc, new_asoc)) {
1857+
struct sctp_chunk *abort;
1858+
1859+
abort = sctp_make_abort(asoc, NULL, sizeof(struct sctp_errhdr));
1860+
if (abort) {
1861+
sctp_init_cause(abort, SCTP_ERROR_RSRC_LOW, 0);
1862+
sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
1863+
}
1864+
sctp_add_cmd_sf(commands, SCTP_CMD_SET_SK_ERR, SCTP_ERROR(ECONNABORTED));
1865+
sctp_add_cmd_sf(commands, SCTP_CMD_ASSOC_FAILED,
1866+
SCTP_PERR(SCTP_ERROR_RSRC_LOW));
1867+
SCTP_INC_STATS(net, SCTP_MIB_ABORTEDS);
1868+
SCTP_DEC_STATS(net, SCTP_MIB_CURRESTAB);
1869+
goto nomem;
1870+
}
1871+
1872+
repl = sctp_make_cookie_ack(asoc, chunk);
18561873
if (!repl)
18571874
goto nomem;
18581875

18591876
/* Report association restart to upper layer. */
18601877
ev = sctp_ulpevent_make_assoc_change(asoc, 0, SCTP_RESTART, 0,
1861-
new_asoc->c.sinit_num_ostreams,
1862-
new_asoc->c.sinit_max_instreams,
1878+
asoc->c.sinit_num_ostreams,
1879+
asoc->c.sinit_max_instreams,
18631880
NULL, GFP_ATOMIC);
18641881
if (!ev)
18651882
goto nomem_ev;
18661883

1867-
/* Update the content of current association. */
1868-
sctp_add_cmd_sf(commands, SCTP_CMD_UPDATE_ASSOC, SCTP_ASOC(new_asoc));
18691884
sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP, SCTP_ULPEVENT(ev));
18701885
if ((sctp_state(asoc, SHUTDOWN_PENDING) ||
18711886
sctp_state(asoc, SHUTDOWN_SENT)) &&

0 commit comments

Comments
 (0)