ppp: error path fixes

index 137f6b9..39e872b 100644
@@ -454,9 +454,12 @@ static void pppcp_send_code_reject(struct pppcp_data *data,
 					guint8 *rejected_packet)
 {
 	struct pppcp_packet *packet;
+	struct pppcp_packet *old_packet =
+				(struct pppcp_packet *) rejected_packet;
 
-	packet = pppcp_packet_new(data, CODE_REJECT,
-			ntohs(((struct pppcp_packet *) rejected_packet)->length));
+	pppcp_trace(data);
+
+	packet = pppcp_packet_new(data, CODE_REJECT, ntohs(old_packet->length));
 
 	/*
 	 * Identifier must be changed for each Code-Reject sent

@@ -468,7 +471,7 @@ static void pppcp_send_code_reject(struct pppcp_data *data,
 	 * truncated if it needs to be to comply with mtu requirement
 	 */
 	memcpy(packet->data, rejected_packet,
-			ntohs(packet->length - CP_HEADER_SZ));
+			ntohs(packet->length) - CP_HEADER_SZ);
 
 	ppp_transmit(data->ppp, pppcp_to_ppp_packet(packet),
 			ntohs(packet->length));

index 39e872b..f83cfba 100644
@@ -1401,6 +1401,10 @@ static guint8 pppcp_process_code_reject(struct pppcp_data *data,
 	 * determine if the code reject is catastrophic or not.
 	 * return RXJ_PLUS if this reject is acceptable, RXJ_MINUS if
 	 * it is catastrophic.
+	 *
+	 * for now we always return RXJ_MINUS.  Any code
+	 * reject will be catastrophic, since we only support the
+	 * bare minimum number of codes necessary to function.
 	 */
 	return RXJ_MINUS;
 }

@@ -1412,6 +1416,10 @@ static guint8 pppcp_process_protocol_reject(struct pppcp_data *data,
 	 * determine if the protocol reject is catastrophic or not.
 	 * return RXJ_PLUS if this reject is acceptable, RXJ_MINUS if
 	 * it is catastrophic.
+	 *
+	 * for now we always return RXJ_MINUS.  Any protocol
+	 * reject will be catastrophic, since we only support the
+	 * bare minimum number of protocols necessary to function.
 	 */
 	return RXJ_MINUS;
 }

index d771c3f..f93b52d 100644
@@ -39,6 +39,11 @@
 #define PPPINITFCS16    0xffff  /* Initial FCS value */
 #define PPPGOODFCS16    0xf0b8  /* Good final FCS value */
 
+struct frame_buffer {
+	gsize len;
+	guint8 bytes[0];
+};
+
 static GList *packet_handlers = NULL;
 
 void ppp_register_packet_handler(struct ppp_packet_handler *handler)

@@ -185,12 +190,12 @@ static gint is_proto_handler(gconstpointer a, gconstpointer b)
 static void ppp_recv(GAtPPP *ppp)
 {
 	GList *list;
-	guint8 *frame;
+	struct frame_buffer *frame;
 
 	/* pop frames off of receive queue */
 	while ((frame = g_queue_pop_head(ppp->recv_queue))) {
-		guint protocol = ppp_proto(frame);
-		guint8 *packet = ppp_info(frame);
+		guint protocol = ppp_proto(frame->bytes);
+		guint8 *packet = ppp_info(frame->bytes);
 		struct ppp_packet_handler *h;
 
 		/*

@@ -203,23 +208,26 @@ static void ppp_recv(GAtPPP *ppp)
 		if (list) {
 			h = list->data;
 			h->handler(h->priv, packet);
-		}
+		} else
+			lcp_protocol_reject(ppp->lcp, frame->bytes, frame->len);
 		g_free(frame);
 	}
 }
 
 /* XXX - Implement PFC and ACFC */
-static guint8 *ppp_decode(GAtPPP *ppp, guint8 *frame)
+static struct frame_buffer *ppp_decode(GAtPPP *ppp, guint8 *frame)
 {
 	guint8 *data;
 	guint pos = 0;
 	int i = 0;
 	int len;
 	guint16 fcs;
+	struct frame_buffer *fb;
 
-	data = g_try_malloc0(ppp->mru + 10);
-	if (!data)
+	fb = g_try_malloc0(sizeof(struct frame_buffer) + ppp->mru + 10);
+	if (!fb)
 		return NULL;
+	data = fb->bytes;
 
 	/* skip the first flag char */
 	pos++;

@@ -237,6 +245,7 @@ static guint8 *ppp_decode(GAtPPP *ppp, guint8 *frame)
 	}
 
 	len = i;
+	fb->len = len;
 
 	/* see if we have a good FCS */
 	fcs = PPPINITFCS16;

@@ -244,16 +253,16 @@ static guint8 *ppp_decode(GAtPPP *ppp, guint8 *frame)
 		fcs = ppp_fcs(fcs, data[i]);
 
 	if (fcs != PPPGOODFCS16) {
-		g_free(data);
+		g_free(fb);
 		return NULL;
 	}
-	return data;
+	return fb;
 }
 
 static void ppp_feed(GAtPPP *ppp, guint8 *data, gsize len)
 {
 	guint pos = 0;
-	guint8 *frame;
+	struct frame_buffer *frame;
 
 	/* collect bytes until we detect we have received a complete frame */
 	/* examine the data.  If we are at the beginning of a new frame,

index 05cd50f..6797603 100644
@@ -156,6 +156,7 @@ void lcp_open(struct pppcp_data *data);
 void lcp_close(struct pppcp_data *data);
 void lcp_establish(struct pppcp_data *data);
 void lcp_terminate(struct pppcp_data *data);
+void lcp_protocol_reject(struct pppcp_data *lcp, guint8 *packet, gsize len);
 void auth_set_credentials(struct auth_data *data, const char *username,
 				const char *passwd);
 void auth_set_proto(struct auth_data *data, guint16 proto, guint8 method);

48 lines not on this page · raw message

index 69676cd..e326b5e 100644
@@ -150,3 +150,5 @@ void pppcp_generate_event(struct pppcp_data *data,
 				enum pppcp_event_type event_type,
 				gpointer event_data, guint data_len);
 void pppcp_process_packet(gpointer priv, guint8 *new_packet);
+void pppcp_send_protocol_reject(struct pppcp_data *data,
+				guint8 *rejected_packet, gsize len);

index 7206b4b..1ccc3cc 100644
@@ -220,6 +220,11 @@ void lcp_free(struct pppcp_data *lcp)
 	pppcp_free(lcp);
 }
 
+void lcp_protocol_reject(struct pppcp_data *lcp, guint8 *packet, gsize len)
+{
+	pppcp_send_protocol_reject(lcp, packet, len);
+}
+
 struct pppcp_data *lcp_new(GAtPPP *ppp)
 {
 	struct pppcp_data *pppcp;

index 263fa8b..d7d70b7 100644
@@ -1140,6 +1140,22 @@ static void remove_config_option(gpointer elem, gpointer user_data)
 	data->config_options = g_list_delete_link(data->config_options, list);
 }
 
+static struct ppp_option *extract_ppp_option(guint8 *packet_data)
+{
+	struct ppp_option *option;
+	guint8 otype = packet_data[0];
+	guint8 olen = packet_data[1];
+
+	option = g_try_malloc0(olen);
+	if (option == NULL)
+		return NULL;
+
+	option->type = otype;
+	option->length = olen;
+	memcpy(option->data, &packet_data[2], olen-2);
+	return option;
+}
+
 static guint8 pppcp_process_configure_request(struct pppcp_data *data,
 					struct pppcp_packet *packet)
 {

@@ -1157,14 +1173,13 @@ static guint8 pppcp_process_configure_request(struct pppcp_data *data,
 	 * check the options.
 	 */
 	while (i < len) {
-		guint8 otype = packet->data[i];
-		guint8 olen = packet->data[i+1];
-		option = g_try_malloc0(olen);
+		option = extract_ppp_option(&packet->data[i]);
 		if (option == NULL)
 			break;
-		option->type = otype;
-		option->length = olen;
-		memcpy(option->data, &packet->data[i+2], olen-2);
+
+		/* skip ahead to the next option */
+		i += option->length;
+
 		if (action->option_scan)
 			rval = action->option_scan(option, data);
 		switch (rval) {

@@ -1182,10 +1197,9 @@ static guint8 pppcp_process_configure_request(struct pppcp_data *data,
 						option);
 			break;
 		case OPTION_ERR:
-			g_printerr("unhandled option type %d\n", otype);
+			g_printerr("unhandled option type %d\n", option->type);
+			g_free(option);
 		}
-		/* skip ahead to the next option */
-		i += olen;
 	}
 
 	/* make sure all required config options were included */

@@ -1242,16 +1256,12 @@ static guint8 pppcp_process_configure_ack(struct pppcp_data *data,
 	 * and apply them.
 	 */
 	while (i < len) {
-		guint8 otype = packet->data[i];
-		guint8 olen = packet->data[i + 1];
-		acked_option = g_try_malloc0(olen);
+		acked_option = extract_ppp_option(&packet->data[i]);
 		if (acked_option == NULL)
 			break;
-		acked_option->type = otype;
-		acked_option->length = olen;
-		memcpy(acked_option->data, &packet->data[i + 2], olen - 2);
 		list = g_list_find_custom(data->config_options,
-				GUINT_TO_POINTER((guint) otype), is_option);
+				GUINT_TO_POINTER((guint) acked_option->type),
+				is_option);
 		if (list) {
 			/*
 			 * once we've applied the option, delete it from

@@ -1266,10 +1276,11 @@ static guint8 pppcp_process_configure_ack(struct pppcp_data *data,
 				g_list_delete_link(data->config_options, list);
 		} else
 			g_printerr("oops -- found acked option %d we didn't request\n", acked_option->type);
-		g_free(acked_option);
 
 		/* skip ahead to the next option */
-		i += olen;
+		i += acked_option->length;
+
+		g_free(acked_option);
 	}
 	return RCA;
 }

@@ -1300,14 +1311,13 @@ static guint8 pppcp_process_configure_nak(struct pppcp_data *data,
 	 * modify a value there, or add a new option.
 	 */
 	while (i < len) {
-		guint8 otype = packet->data[i];
-		guint8 olen = packet->data[i+1];
-		naked_option = g_try_malloc0(olen);
+		naked_option = extract_ppp_option(&packet->data[i]);
 		if (naked_option == NULL)
 			break;
-		naked_option->type = otype;
-		naked_option->length = olen;
-		memcpy(naked_option->data, &packet->data[i + 2], olen - 2);
+
+		/* skip ahead to the next option */
+		i += naked_option->length;
+
 		if (action->option_scan)
 			rval = action->option_scan(naked_option, data);
 		if (rval == OPTION_ACCEPT) {

@@ -1316,7 +1326,8 @@ static guint8 pppcp_process_configure_nak(struct pppcp_data *data,
 			 * match.
 			 */
 			list = g_list_find_custom(data->config_options,
-				GUINT_TO_POINTER((guint) otype), is_option);
+				GUINT_TO_POINTER((guint) naked_option->type),
+				is_option);
 			if (list) {
 				/* modify current option value to match */
 				config_option = list->data;

@@ -1326,10 +1337,11 @@ static guint8 pppcp_process_configure_nak(struct pppcp_data *data,
 				 * we need to reallocate
 				 */
 				if ((config_option->length ==
-					naked_option->length) && (olen - 2)) {
+					naked_option->length) &&
+							(naked_option - 2)) {
 						memcpy(config_option->data,
-							naked_option->data,
-							olen - 2);
+						   naked_option->data,
+						   naked_option->length - 2);
 				} else {
 					/* XXX implement this */
 					g_printerr("uh oh, option value doesn't match\n");

@@ -1344,9 +1356,6 @@ static guint8 pppcp_process_configure_nak(struct pppcp_data *data,
 			g_printerr("oops, option wasn't acceptable\n");
 			g_free(naked_option);
 		}
-
-		/* skip ahead to the next option */
-		i += olen;
 	}
 	return RCN;
 }

index d7d70b7..ac1a37d 100644
@@ -1363,22 +1363,49 @@ static guint8 pppcp_process_configure_nak(struct pppcp_data *data,
 static guint8 pppcp_process_configure_reject(struct pppcp_data *data,
 					struct pppcp_packet *packet)
 {
+	guint len;
+	GList *list;
+	struct ppp_option *rejected_option;
+	guint i = 0;
+
+	len = ntohs(packet->length) - CP_HEADER_SZ;
+
 	/*
 	 * make sure identifier matches that of last sent configure
 	 * request
 	 */
-	if (packet->identifier == data->config_identifier) {
-		/*
-		 * check to see which options were rejected
-		 * Rejected options must be a subset of requested
-		 * options.
-		 *
-		 * when a new configure-request is sent, we may
-		 * not request any of these options be negotiated
-		 */
-		return RCN;
+	if (packet->identifier != data->config_identifier)
+		return 0;
+
+	/*
+	 * check to see which options were rejected
+	 * Rejected options must be a subset of requested
+	 * options.
+	 *
+	 * when a new configure-request is sent, we may
+	 * not request any of these options be negotiated
+	 */
+	while (i < len) {
+		rejected_option = extract_ppp_option(&packet->data[i]);
+		if (rejected_option == NULL)
+			break;
+
+		/* skip ahead to the next option */
+		i += rejected_option->length;
+
+		/* find this option in our config options list */
+		list = g_list_find_custom(data->config_options,
+			GUINT_TO_POINTER((guint) rejected_option->type),
+			is_option);
+		if (list) {
+			/* delete this config option */
+			g_free(list->data);
+			data->config_options =
+				g_list_delete_link(data->config_options, list);
+		}
+		g_free(rejected_option);
 	}
-	return 0;
+	return RCN;
 }
 
 static guint8 pppcp_process_terminate_request(struct pppcp_data *data,