[SCTP]: sctp_process_init can fail; cleanup and bail on errors. (jgrimm)

net/sctp/associola.c

 /* SCTP kernel reference Implementation
  * Copyright (c) 1999-2000 Cisco, Inc.
  * Copyright (c) 1999-2001 Motorola, Inc.
- * Copyright (c) 2001 International Business Machines Corp.
+ * Copyright (c) 2001-2002 International Business Machines Corp.
  * Copyright (c) 2001 Intel Corp.
  * Copyright (c) 2001 La Monte H.P. Yarroll
  * 
@@ -906,17 +906,13 @@ static void sctp_assoc_bh_rcv(sctp_association_t *asoc)
 		 * the incoming chunk.  If so, get out of the while loop.
 		 */
 		if (!sctp_id2assoc(sk, associd))
-			goto out;
+			break;
 
-		if (error != 0)
-			goto err_out;
+		/* If there is an error on chunk, discard this packet. */
+		if (error && chunk)
+			chunk->pdiscard = 1;
 	}
 
-err_out:
-	/* Is this the right way to pass errors up to the ULP?  */
-	if (error)
-		sk->err = -error;
-out:
 }
 
 /* This routine moves an association from its old sk to a new sk.  */

net/sctp/endpointola.c

 /* SCTP kernel reference Implementation
  * Copyright (c) 1999-2000 Cisco, Inc.
  * Copyright (c) 1999-2001 Motorola, Inc.
- * Copyright (c) 2001 International Business Machines, Corp.
+ * Copyright (c) 2001-2002 International Business Machines, Corp.
  * Copyright (c) 2001 Intel Corp.
  * Copyright (c) 2001 Nokia, Inc.
  * Copyright (c) 2001 La Monte H.P. Yarroll
@@ -316,13 +316,13 @@ static void sctp_endpoint_bh_rcv(sctp_endpoint_t *ep)
 	int error = 0;
 
 	if (ep->base.dead)
-		goto out;
+		return;
 
 	asoc = NULL;
 	inqueue = &ep->base.inqueue;
 	sk = ep->base.sk;
 
-	while (NULL != (chunk = sctp_pop_inqueue(inqueue))) {
+	while (NULL != (chunk = sctp_pop_inqueue(inqueue))) {		
 		subtype.chunk = chunk->chunk_hdr->type;
 
 		/* We might have grown an association since last we
@@ -350,25 +350,16 @@ static void sctp_endpoint_bh_rcv(sctp_endpoint_t *ep)
 		if (chunk->transport)
 			chunk->transport->last_time_heard = jiffies;
 
-		/* FIX ME We really would rather NOT have to use
-		 * GFP_ATOMIC.
-		 */
 		error = sctp_do_sm(SCTP_EVENT_T_CHUNK, subtype, state,
                                    ep, asoc, chunk, GFP_ATOMIC);
 
-		if (error != 0)
-			goto err_out;
+		if (error && chunk)
+			chunk->pdiscard = 1;
 
 		/* Check to see if the endpoint is freed in response to
 		 * the incoming chunk. If so, get out of the while loop.
 		 */
 		if (!sctp_sk(sk)->ep)
-			goto out;
+			break;
 	}
-
-err_out:
-	/* Is this the right way to pass errors up to the ULP?  */
-	if (error)
-		ep->base.sk->err = -error;
-out:
 }

net/sctp/sm_sideeffect.c

@@ -69,10 +69,10 @@ static void sctp_do_8_2_transport_strike(sctp_association_t *asoc,
 static void sctp_cmd_init_failed(sctp_cmd_seq_t *, sctp_association_t *asoc);
 static void sctp_cmd_assoc_failed(sctp_cmd_seq_t *, sctp_association_t *asoc,
 				  sctp_event_t event_type, sctp_chunk_t *chunk);
-static void sctp_cmd_process_init(sctp_cmd_seq_t *, sctp_association_t *asoc,
-				  sctp_chunk_t *chunk,
-				  sctp_init_chunk_t *peer_init,
-				  int priority);
+static int sctp_cmd_process_init(sctp_cmd_seq_t *, sctp_association_t *asoc,
+				 sctp_chunk_t *chunk,
+				 sctp_init_chunk_t *peer_init,
+				 int priority);
 static void sctp_cmd_hb_timers_start(sctp_cmd_seq_t *, sctp_association_t *);
 static void sctp_cmd_hb_timers_update(sctp_cmd_seq_t *, sctp_association_t *,
 				      sctp_transport_t *);
@@ -343,9 +343,14 @@ int sctp_cmd_interpreter(sctp_event_t event_type, sctp_subtype_t subtype,
 			break;
 
 		case SCTP_CMD_PEER_INIT:
-			/* Process a unified INIT from the peer.  */
-			sctp_cmd_process_init(commands, asoc, chunk,
-					      command->obj.ptr, priority);
+			/* Process a unified INIT from the peer.
+			 * Note: Only used during INIT-ACK processing.  If
+			 * there is an error just return to the outter
+			 * layer which will bail.
+			 */
+			error = sctp_cmd_process_init(commands, asoc, chunk,
+						      command->obj.ptr,
+						      priority);
 			break;
 
 		case SCTP_CMD_GEN_COOKIE_ECHO:
@@ -564,7 +569,7 @@ int sctp_cmd_interpreter(sctp_event_t event_type, sctp_subtype_t subtype,
 		case SCTP_CMD_HB_TIMERS_START:
 			sctp_cmd_hb_timers_start(commands, asoc);
 			break;
-	
+
 		case SCTP_CMD_HB_TIMERS_UPDATE:
 			t = command->obj.transport;
 			sctp_cmd_hb_timers_update(commands, asoc, t);
@@ -595,6 +600,8 @@ int sctp_cmd_interpreter(sctp_event_t event_type, sctp_subtype_t subtype,
 			       command->verb, command->obj.ptr);
 			break;
 		};
+		if (error)
+			return error;
 	}
 
 	return error;
@@ -657,7 +664,7 @@ static sctp_chunk_t *sctp_do_ecn_ecne_work(sctp_association_t *asoc,
 	}
 
 	/* Always try to quiet the other end.  In case of lost CWR,
-	 * resend last_cwr_tsn.  
+	 * resend last_cwr_tsn.
 	 */
 	repl = sctp_make_cwr(asoc, asoc->last_cwr_tsn, chunk);
 
@@ -1067,22 +1074,32 @@ static void sctp_cmd_assoc_failed(sctp_cmd_seq_t *commands,
 }
 
 /* Process an init chunk (may be real INIT/INIT-ACK or an embedded INIT
- * inside the cookie.
+ * inside the cookie.  In reality, this is only used for INIT-ACK processing
+ * since all other cases use "temporary" associations and can do all
+ * their work in statefuns directly. 
  */
-static void sctp_cmd_process_init(sctp_cmd_seq_t *commands,
-				  sctp_association_t *asoc,
-				  sctp_chunk_t *chunk,
-				  sctp_init_chunk_t *peer_init,
-				  int priority)
+static int sctp_cmd_process_init(sctp_cmd_seq_t *commands,
+				 sctp_association_t *asoc,
+				 sctp_chunk_t *chunk,
+				 sctp_init_chunk_t *peer_init,
+				 int priority)
 {
-	/* The command sequence holds commands assuming that the
-	 * processing will happen successfully.  If this is not the
-	 * case, rewind the sequence and add appropriate  error handling
-	 * to the sequence.
+	int error;
+
+	/* We only process the init as a sideeffect in a single
+	 * case.   This is when we process the INIT-ACK.   If we
+	 * fail during INIT processing (due to malloc problems),
+	 * just return the error and stop processing the stack.
 	 */
-	sctp_process_init(asoc, chunk->chunk_hdr->type,
-			  sctp_source(chunk), peer_init,
-			  priority);
+
+	if (!sctp_process_init(asoc, chunk->chunk_hdr->type,
+			       sctp_source(chunk), peer_init,
+			       priority))
+		error = -ENOMEM;
+	else
+		error = 0;
+
+	return error;
 }
 
 /* Helper function to break out starting up of heartbeat timers.  */

net/sctp/sm_statefuns.c

@@ -242,13 +242,12 @@ sctp_disposition_t sctp_sf_do_5_1B_init(const sctp_endpoint_t *ep,
 	if (!new_asoc)
 		goto nomem;
 
-	/* FIXME: sctp_process_init can fail, but there is no
-	 * status nor handling.
-	 */
-	sctp_process_init(new_asoc, chunk->chunk_hdr->type,
-			  sctp_source(chunk),
-			  (sctp_init_chunk_t *)chunk->chunk_hdr,
-			  GFP_ATOMIC);
+	/* The call, sctp_process_init(), can fail on memory allocation.  */
+	if (!sctp_process_init(new_asoc, chunk->chunk_hdr->type,
+			       sctp_source(chunk),
+			       (sctp_init_chunk_t *)chunk->chunk_hdr,
+			       GFP_ATOMIC))
+		goto nomem_init;
 
 	sctp_add_cmd_sf(commands, SCTP_CMD_NEW_ASOC, SCTP_ASOC(new_asoc));
 
@@ -301,10 +300,10 @@ sctp_disposition_t sctp_sf_do_5_1B_init(const sctp_endpoint_t *ep,
 	return SCTP_DISPOSITION_DELETE_TCB;
 
 nomem_ack:
-	sctp_association_free(new_asoc);
 	if (err_chunk)
 		sctp_free_chunk(err_chunk);
-
+nomem_init:
+	sctp_association_free(new_asoc);
 nomem:
 	return SCTP_DISPOSITION_NOMEM;
 }
@@ -562,9 +561,11 @@ sctp_disposition_t sctp_sf_do_5_1D_ce(const sctp_endpoint_t *ep,
 	 * effects--it is safe to run them here.
 	 */
 	peer_init = &chunk->subh.cookie_hdr->c.peer_init[0];
-	sctp_process_init(new_asoc, chunk->chunk_hdr->type,
-			  &chunk->subh.cookie_hdr->c.peer_addr, peer_init,
-			  GFP_ATOMIC);
+
+	if (!sctp_process_init(new_asoc, chunk->chunk_hdr->type,
+			       &chunk->subh.cookie_hdr->c.peer_addr,
+			       peer_init, GFP_ATOMIC))
+		goto nomem_init;
 
 	repl = sctp_make_cookie_ack(new_asoc, chunk);
 	if (!repl)
@@ -591,10 +592,9 @@ sctp_disposition_t sctp_sf_do_5_1D_ce(const sctp_endpoint_t *ep,
 
 nomem_ev:
 	sctp_free_chunk(repl);
-
 nomem_repl:
+nomem_init:
 	sctp_association_free(new_asoc);
-
 nomem:
 	return SCTP_DISPOSITION_NOMEM;
 }
@@ -674,7 +674,7 @@ sctp_disposition_t sctp_sf_heartbeat(const sctp_endpoint_t *ep,
 	sctp_chunk_t *reply;
 	sctp_sender_hb_info_t hbinfo;
 	size_t paylen = 0;
-	
+
 	hbinfo.param_hdr.type = SCTP_PARAM_HEARTBEAT_INFO;
 	hbinfo.param_hdr.length = htons(sizeof(sctp_sender_hb_info_t));
 	hbinfo.daddr = transport->ipaddr;
@@ -730,7 +730,7 @@ sctp_disposition_t sctp_sf_sendbeat_8_3(const sctp_endpoint_t *ep,
 	}
 	sctp_add_cmd_sf(commands, SCTP_CMD_HB_TIMERS_UPDATE,
 			SCTP_TRANSPORT(transport));
-	
+
         return SCTP_DISPOSITION_CONSUME;
 }
 
@@ -1140,8 +1140,13 @@ static sctp_disposition_t sctp_sf_do_unexpected_init(
 	 * Verification Tag and Peers Verification tag into a reserved
 	 * place (local tie-tag and per tie-tag) within the state cookie.
 	 */
-	sctp_process_init(new_asoc, chunk->chunk_hdr->type, sctp_source(chunk),
-			  (sctp_init_chunk_t *)chunk->chunk_hdr, GFP_ATOMIC);
+	if (!sctp_process_init(new_asoc, chunk->chunk_hdr->type,
+			       sctp_source(chunk),
+			       (sctp_init_chunk_t *)chunk->chunk_hdr,
+			       GFP_ATOMIC)) {
+		retval = SCTP_DISPOSITION_NOMEM;
+		goto nomem_init;
+	}
 
 	/* Make sure no new addresses are being added during the
 	 * restart.   Do not do this check for COOKIE-WAIT state,
@@ -1212,6 +1217,7 @@ static sctp_disposition_t sctp_sf_do_unexpected_init(
 nomem:
 	retval = SCTP_DISPOSITION_NOMEM;
 	goto cleanup;
+nomem_init:
 cleanup_asoc:
 	sctp_association_free(new_asoc);
 	goto cleanup;
@@ -1341,15 +1347,16 @@ static sctp_disposition_t sctp_sf_do_dupcook_a(const sctp_endpoint_t *ep,
 	 * side effects--it is safe to run them here.
 	 */
 	peer_init = &chunk->subh.cookie_hdr->c.peer_init[0];
-	sctp_process_init(new_asoc, chunk->chunk_hdr->type,
-			  sctp_source(chunk), peer_init, GFP_ATOMIC);
+
+	if (!sctp_process_init(new_asoc, chunk->chunk_hdr->type,
+			       sctp_source(chunk), peer_init, GFP_ATOMIC))
+		goto nomem;
 
 	/* Make sure no new addresses are being added during the
 	 * restart.  Though this is a pretty complicated attack
 	 * since you'd have to get inside the cookie.
 	 */
 	if (!sctp_sf_check_restart_addrs(new_asoc, asoc, chunk, commands)) {
-		printk("cookie echo check\n");
 		return SCTP_DISPOSITION_CONSUME;
 	}
 
@@ -1406,8 +1413,9 @@ static sctp_disposition_t sctp_sf_do_dupcook_b(const sctp_endpoint_t *ep,
 	 * side effects--it is safe to run them here.
 	 */
 	peer_init = &chunk->subh.cookie_hdr->c.peer_init[0];
-	sctp_process_init(new_asoc, chunk->chunk_hdr->type,
-			  sctp_source(chunk), peer_init, GFP_ATOMIC);
+	if (!sctp_process_init(new_asoc, chunk->chunk_hdr->type,
+			       sctp_source(chunk), peer_init, GFP_ATOMIC))
+		goto nomem;
 
 	/* Update the content of current association.  */
 	sctp_add_cmd_sf(commands, SCTP_CMD_UPDATE_ASSOC, SCTP_ASOC(new_asoc));
@@ -3671,7 +3679,7 @@ sctp_disposition_t sctp_sf_shutdown_ack_sent_prm_abort(
 	return sctp_sf_shutdown_sent_prm_abort(ep, asoc, type, arg, commands);
 }
 
-/* 
+/*
  * Process the REQUESTHEARTBEAT primitive
  *
  * 10.1 ULP-to-SCTP