sctp: fix use-after-free in pr_debug statement

Dmitry Vyukov reported a use-after-free in the code expanded by the
macro debug_post_sfx, which is caused by the use of the asoc pointer
after it was freed within sctp_side_effect() scope.

This patch fixes it by allowing sctp_side_effect to clear that asoc
pointer when the TCB is freed.

As Vlad explained, we also have to cover the SCTP_DISPOSITION_ABORT case
because it will trigger DELETE_TCB too on that same loop.

Also, there were places issuing SCTP_CMD_INIT_FAILED and ASSOC_FAILED
but returning SCTP_DISPOSITION_CONSUME, which would fool the scheme
above. Fix it by returning SCTP_DISPOSITION_ABORT instead.

The macro is already prepared to handle such NULL pointer.

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 6098d4c42fa91287d3cde36ac05d860f76d4fe32..be23d5c2074fa49801a50028da27d11dd8fef330 100644 (file)
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -63,7 +63,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
 static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype,
                             sctp_state_t state,
                             struct sctp_endpoint *ep,
-                            struct sctp_association *asoc,
+                            struct sctp_association **asoc,
                             void *event_arg,
                             sctp_disposition_t status,
                             sctp_cmd_seq_t *commands,
@@ -1123,7 +1123,7 @@ int sctp_do_sm(struct net *net, sctp_event_t event_type, sctp_subtype_t subtype,
        debug_post_sfn();
 
        error = sctp_side_effects(event_type, subtype, state,
-                                 ep, asoc, event_arg, status,
+                                 ep, &asoc, event_arg, status,
                                  &commands, gfp);
        debug_post_sfx();
 
@@ -1136,7 +1136,7 @@ int sctp_do_sm(struct net *net, sctp_event_t event_type, sctp_subtype_t subtype,
 static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype,
                             sctp_state_t state,
                             struct sctp_endpoint *ep,
-                            struct sctp_association *asoc,
+                            struct sctp_association **asoc,
                             void *event_arg,
                             sctp_disposition_t status,
                             sctp_cmd_seq_t *commands,
@@ -1151,7 +1151,7 @@ static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype,
         * disposition SCTP_DISPOSITION_CONSUME.
         */
        if (0 != (error = sctp_cmd_interpreter(event_type, subtype, state,
-                                              ep, asoc,
+                                              ep, *asoc,
                                               event_arg, status,
                                               commands, gfp)))
                goto bail;
@@ -1174,11 +1174,12 @@ static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype,
                break;
 
        case SCTP_DISPOSITION_DELETE_TCB:
+       case SCTP_DISPOSITION_ABORT:
                /* This should now be a command. */
+               *asoc = NULL;
                break;
 
        case SCTP_DISPOSITION_CONSUME:
-       case SCTP_DISPOSITION_ABORT:
                /*
                 * We should no longer have much work to do here as the
                 * real work has been done as explicit commands above.

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 22c2bf367d7e8c7025065f33eabfd7e93a7f4021..f1f08c8f277bd8719299d1ed21eb23e36d55f7e2 100644 (file)
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -2976,7 +2976,7 @@ sctp_disposition_t sctp_sf_eat_data_6_2(struct net *net,
                SCTP_INC_STATS(net, SCTP_MIB_IN_DATA_CHUNK_DISCARDS);
                goto discard_force;
        case SCTP_IERROR_NO_DATA:
-               goto consume;
+               return SCTP_DISPOSITION_ABORT;
        case SCTP_IERROR_PROTO_VIOLATION:
                return sctp_sf_abort_violation(net, ep, asoc, chunk, commands,
                        (u8 *)chunk->subh.data_hdr, sizeof(sctp_datahdr_t));
@@ -3043,9 +3043,6 @@ discard_noforce:
                sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, force);
 
        return SCTP_DISPOSITION_DISCARD;
-consume:
-       return SCTP_DISPOSITION_CONSUME;
-
 }
 
 /*
@@ -3093,7 +3090,7 @@ sctp_disposition_t sctp_sf_eat_data_fast_4_4(struct net *net,
        case SCTP_IERROR_BAD_STREAM:
                break;
        case SCTP_IERROR_NO_DATA:
-               goto consume;
+               return SCTP_DISPOSITION_ABORT;
        case SCTP_IERROR_PROTO_VIOLATION:
                return sctp_sf_abort_violation(net, ep, asoc, chunk, commands,
                        (u8 *)chunk->subh.data_hdr, sizeof(sctp_datahdr_t));
@@ -3119,7 +3116,6 @@ sctp_disposition_t sctp_sf_eat_data_fast_4_4(struct net *net,
                                SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN));
        }
 
-consume:
        return SCTP_DISPOSITION_CONSUME;
 }
 
@@ -4825,9 +4821,6 @@ sctp_disposition_t sctp_sf_do_9_1_prm_abort(
         * if necessary to fill gaps.
         */
        struct sctp_chunk *abort = arg;
-       sctp_disposition_t retval;
-
-       retval = SCTP_DISPOSITION_CONSUME;
 
        if (abort)
                sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
@@ -4845,7 +4838,7 @@ sctp_disposition_t sctp_sf_do_9_1_prm_abort(
        SCTP_INC_STATS(net, SCTP_MIB_ABORTEDS);
        SCTP_DEC_STATS(net, SCTP_MIB_CURRESTAB);
 
-       return retval;
+       return SCTP_DISPOSITION_ABORT;
 }
 
 /* We tried an illegal operation on an association which is closed.  */
@@ -4960,12 +4953,10 @@ sctp_disposition_t sctp_sf_cookie_wait_prm_abort(
        sctp_cmd_seq_t *commands)
 {
        struct sctp_chunk *abort = arg;
-       sctp_disposition_t retval;
 
        /* Stop T1-init timer */
        sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_STOP,
                        SCTP_TO(SCTP_EVENT_TIMEOUT_T1_INIT));
-       retval = SCTP_DISPOSITION_CONSUME;
 
        if (abort)
                sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
@@ -4985,7 +4976,7 @@ sctp_disposition_t sctp_sf_cookie_wait_prm_abort(
        sctp_add_cmd_sf(commands, SCTP_CMD_INIT_FAILED,
                        SCTP_PERR(SCTP_ERROR_USER_ABORT));
 
-       return retval;
+       return SCTP_DISPOSITION_ABORT;
 }
 
 /*