diff mbox

sctp: Use correct sideffect command in duplicate cookie handling

Message ID 1363139603-14042-1-git-send-email-vyasevich@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Vladislav Yasevich March 13, 2013, 1:53 a.m. UTC
When SCTP is done processing a duplicate cookie chunk, it tries
to delete a newly created association.  For that, it has to set
the right association for the side-effect processing to work.
However, when it uses the SCTP_CMD_NEW_ASOC command, that performs
more work then really needed (like hashing the associationa and
assigning it an id) and there is no point to do that only to
delete the association as a next step.  In fact, it also creates
an impossible condition where an association may be found by
the getsockopt() call, and that association is empty.  This
causes a crash in some sctp getsockopts.

The solution is rather simple.  We simply use SCTP_CMD_SET_ASOC
command that doesn't have all the overhead and does exactly
what we need.

Reported-by: Karl Heiss <kheiss@gmail.com>
Tested-by: Karl Heiss <kheiss@gmail.com>
CC: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>
---
 net/sctp/sm_statefuns.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Neil Horman March 13, 2013, 1:29 p.m. UTC | #1
On Tue, Mar 12, 2013 at 09:53:23PM -0400, Vlad Yasevich wrote:
> When SCTP is done processing a duplicate cookie chunk, it tries
> to delete a newly created association.  For that, it has to set
> the right association for the side-effect processing to work.
> However, when it uses the SCTP_CMD_NEW_ASOC command, that performs
> more work then really needed (like hashing the associationa and
> assigning it an id) and there is no point to do that only to
> delete the association as a next step.  In fact, it also creates
> an impossible condition where an association may be found by
> the getsockopt() call, and that association is empty.  This
> causes a crash in some sctp getsockopts.
> 
> The solution is rather simple.  We simply use SCTP_CMD_SET_ASOC
> command that doesn't have all the overhead and does exactly
> what we need.
> 
> Reported-by: Karl Heiss <kheiss@gmail.com>
> Tested-by: Karl Heiss <kheiss@gmail.com>
> CC: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>
> ---
>  net/sctp/sm_statefuns.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 5131fcf..de1a013 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -2082,7 +2082,7 @@ sctp_disposition_t sctp_sf_do_5_2_4_dupcook(struct net *net,
>  	}
>  
>  	/* Delete the tempory new association. */
> -	sctp_add_cmd_sf(commands, SCTP_CMD_NEW_ASOC, SCTP_ASOC(new_asoc));
> +	sctp_add_cmd_sf(commands, SCTP_CMD_SET_ASOC, SCTP_ASOC(new_asoc));
>  	sctp_add_cmd_sf(commands, SCTP_CMD_DELETE_TCB, SCTP_NULL());
>  
>  	/* Restore association pointer to provide SCTP command interpeter
> -- 
> 1.7.7.6
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller March 13, 2013, 2:04 p.m. UTC | #2
From: Vlad Yasevich <vyasevich@gmail.com>
Date: Tue, 12 Mar 2013 21:53:23 -0400

> When SCTP is done processing a duplicate cookie chunk, it tries
> to delete a newly created association.  For that, it has to set
> the right association for the side-effect processing to work.
> However, when it uses the SCTP_CMD_NEW_ASOC command, that performs
> more work then really needed (like hashing the associationa and
> assigning it an id) and there is no point to do that only to
> delete the association as a next step.  In fact, it also creates
> an impossible condition where an association may be found by
> the getsockopt() call, and that association is empty.  This
> causes a crash in some sctp getsockopts.
> 
> The solution is rather simple.  We simply use SCTP_CMD_SET_ASOC
> command that doesn't have all the overhead and does exactly
> what we need.
> 
> Reported-by: Karl Heiss <kheiss@gmail.com>
> Tested-by: Karl Heiss <kheiss@gmail.com>
> CC: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 5131fcf..de1a013 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -2082,7 +2082,7 @@  sctp_disposition_t sctp_sf_do_5_2_4_dupcook(struct net *net,
 	}
 
 	/* Delete the tempory new association. */
-	sctp_add_cmd_sf(commands, SCTP_CMD_NEW_ASOC, SCTP_ASOC(new_asoc));
+	sctp_add_cmd_sf(commands, SCTP_CMD_SET_ASOC, SCTP_ASOC(new_asoc));
 	sctp_add_cmd_sf(commands, SCTP_CMD_DELETE_TCB, SCTP_NULL());
 
 	/* Restore association pointer to provide SCTP command interpeter