| Message ID | fd17356abe49713ded425250cc1ae51e9f5846c6.1696172325.git.lucien.xin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2222a78075f0c19ca18db53fd6623afb4aff602d |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] sctp: update transport state when processing a dupcook packet | expand |
On Sun, Oct 01, 2023 at 10:58:45AM -0400, Xin Long wrote: > During the 4-way handshake, the transport's state is set to ACTIVE in > sctp_process_init() when processing INIT_ACK chunk on client or > COOKIE_ECHO chunk on server. > > In the collision scenario below: > > 192.168.1.2 > 192.168.1.1: sctp (1) [INIT] [init tag: 3922216408] > 192.168.1.1 > 192.168.1.2: sctp (1) [INIT] [init tag: 144230885] > 192.168.1.2 > 192.168.1.1: sctp (1) [INIT ACK] [init tag: 3922216408] > 192.168.1.1 > 192.168.1.2: sctp (1) [COOKIE ECHO] > 192.168.1.2 > 192.168.1.1: sctp (1) [COOKIE ACK] > 192.168.1.1 > 192.168.1.2: sctp (1) [INIT ACK] [init tag: 3914796021] > > when processing COOKIE_ECHO on 192.168.1.2, as it's in COOKIE_WAIT state, > sctp_sf_do_dupcook_b() is called by sctp_sf_do_5_2_4_dupcook() where it > creates a new association and sets its transport to ACTIVE then updates > to the old association in sctp_assoc_update(). > > However, in sctp_assoc_update(), it will skip the transport update if it > finds a transport with the same ipaddr already existing in the old asoc, > and this causes the old asoc's transport state not to move to ACTIVE > after the handshake. > > This means if DATA retransmission happens at this moment, it won't be able > to enter PF state because of the check 'transport->state == SCTP_ACTIVE' > in sctp_do_8_2_transport_strike(). > > This patch fixes it by updating the transport in sctp_assoc_update() with > sctp_assoc_add_peer() where it updates the transport state if there is > already a transport with the same ipaddr exists in the old asoc. Hi Xin Long, I wonder if this warrants a fixes tag, and if so, perhaps: Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Xin Long <lucien.xin@gmail.com> Reviewed-by: Simon Horman <horms@kernel.org>
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Sun, 1 Oct 2023 10:58:45 -0400 you wrote: > During the 4-way handshake, the transport's state is set to ACTIVE in > sctp_process_init() when processing INIT_ACK chunk on client or > COOKIE_ECHO chunk on server. > > In the collision scenario below: > > 192.168.1.2 > 192.168.1.1: sctp (1) [INIT] [init tag: 3922216408] > 192.168.1.1 > 192.168.1.2: sctp (1) [INIT] [init tag: 144230885] > 192.168.1.2 > 192.168.1.1: sctp (1) [INIT ACK] [init tag: 3922216408] > 192.168.1.1 > 192.168.1.2: sctp (1) [COOKIE ECHO] > 192.168.1.2 > 192.168.1.1: sctp (1) [COOKIE ACK] > 192.168.1.1 > 192.168.1.2: sctp (1) [INIT ACK] [init tag: 3914796021] > > [...] Here is the summary with links: - [net] sctp: update transport state when processing a dupcook packet https://git.kernel.org/netdev/net/c/2222a78075f0 You are awesome, thank you!
diff --git a/net/sctp/associola.c b/net/sctp/associola.c index 796529167e8d..c45c192b7878 100644 --- a/net/sctp/associola.c +++ b/net/sctp/associola.c @@ -1159,8 +1159,7 @@ int sctp_assoc_update(struct sctp_association *asoc, /* Add any peer addresses from the new association. */ list_for_each_entry(trans, &new->peer.transport_addr_list, transports) - if (!sctp_assoc_lookup_paddr(asoc, &trans->ipaddr) && - !sctp_assoc_add_peer(asoc, &trans->ipaddr, + if (!sctp_assoc_add_peer(asoc, &trans->ipaddr, GFP_ATOMIC, trans->state)) return -ENOMEM;
During the 4-way handshake, the transport's state is set to ACTIVE in sctp_process_init() when processing INIT_ACK chunk on client or COOKIE_ECHO chunk on server. In the collision scenario below: 192.168.1.2 > 192.168.1.1: sctp (1) [INIT] [init tag: 3922216408] 192.168.1.1 > 192.168.1.2: sctp (1) [INIT] [init tag: 144230885] 192.168.1.2 > 192.168.1.1: sctp (1) [INIT ACK] [init tag: 3922216408] 192.168.1.1 > 192.168.1.2: sctp (1) [COOKIE ECHO] 192.168.1.2 > 192.168.1.1: sctp (1) [COOKIE ACK] 192.168.1.1 > 192.168.1.2: sctp (1) [INIT ACK] [init tag: 3914796021] when processing COOKIE_ECHO on 192.168.1.2, as it's in COOKIE_WAIT state, sctp_sf_do_dupcook_b() is called by sctp_sf_do_5_2_4_dupcook() where it creates a new association and sets its transport to ACTIVE then updates to the old association in sctp_assoc_update(). However, in sctp_assoc_update(), it will skip the transport update if it finds a transport with the same ipaddr already existing in the old asoc, and this causes the old asoc's transport state not to move to ACTIVE after the handshake. This means if DATA retransmission happens at this moment, it won't be able to enter PF state because of the check 'transport->state == SCTP_ACTIVE' in sctp_do_8_2_transport_strike(). This patch fixes it by updating the transport in sctp_assoc_update() with sctp_assoc_add_peer() where it updates the transport state if there is already a transport with the same ipaddr exists in the old asoc. Signed-off-by: Xin Long <lucien.xin@gmail.com> --- net/sctp/associola.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)