Commit 649621e3 authored by Marcelo Ricardo Leitner's avatar Marcelo Ricardo Leitner Committed by David S. Miller
Browse files

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: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarMarcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Acked-by: default avatarVlad Yasevich <vyasevich@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 366ce603

net/sctp/sm_sideeffect.c

+6 −5
Original line number Diff line number Diff line
@@ -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.

net/sctp/sm_statefuns.c

+4 −13
Original line number Diff line number Diff line
@@ -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;

}



/*

CRA Git | Maintained and supported by SUSTech CRA and CCSE