[Openswan dev] ARM unaligned bug

[Paul Wouters]

On Wed, 20 Jan 2010, Albert Veli wrote:

Albert,

Could you confirm the following patch works on your ARM?

Thanks for your time! And thanks to Hugh for the patch!

Paul
-------------- next part --------------
commit fa384bccde5259d440f552da42a7183cd92b6eeb
Author: Paul Wouters <paul at xelerance.com>
Date:   Wed Jan 20 21:58:27 2010 -0500

    out_modify_previous_np() made assumptions that network order and
    byte order would be the same. Patch by dhr.

diff --git a/include/packet.h b/include/packet.h
index 87bc118..5f2f8b4 100644
--- a/include/packet.h
+++ b/include/packet.h
@@ -166,6 +166,12 @@ extern void DBG_print_struct(const char *label, const void *struct_ptr,
  * require them to be zero).
  */
 
+#define NSIZEOF_isakmp_hdr      20      /* on-the-wire sizeof struct isakmpg_hdr */
+#define NOFFSETOF_isa_np        8       /* on-the-wire offset of isa_np (one octet) */
+#define NOFFSETOF_isag_length   2       /* on-the-wire offset of isag_length (two octets, network order */
+#define NOFFSETOF_isag_np       0       /* on-the-wire offset of isag_np (one octet) */
+#define NSIZEOF_isakmp_generic  4       /* on-the-wire sizeof isakmp_generic) */
+
 struct isakmp_hdr
 {
     u_int8_t    isa_icookie[COOKIE_SIZE];
diff --git a/lib/libpluto/packet.c b/lib/libpluto/packet.c
index f95c190..3405259 100644
--- a/lib/libpluto/packet.c
+++ b/lib/libpluto/packet.c
@@ -1658,34 +1658,44 @@ out_struct(const void *struct_ptr, struct_desc *sd
     return FALSE;
 }
 
+/* Find last complete top-level payload and change its np
+ *  * Note: we must deal with payloads already formatted for the network.
+ *  _*_Note:_we_don't_think_a_FALSE_return_should_happen_but_old_routine_did.
+ *   */
 bool
 out_modify_previous_np(u_int8_t np, pb_stream *outs)
 {
-	size_t len = (outs->cur - outs->start), offset;
-	if (len < sizeof(struct isakmp_hdr)) {
-		return FALSE;
-	}
-	else if (len == sizeof(struct isakmp_hdr)) {
-		struct isakmp_hdr *hdr = (struct isakmp_hdr *)outs->start;
-		hdr->isa_np = np;
-		return TRUE;
-	}
-	else {
-		struct isakmp_generic *hdr;
-		for (offset = sizeof(struct isakmp_hdr); offset < len ;
-			offset += ntohs(hdr->isag_length)) {
-			if ((len - offset) < sizeof(struct isakmp_generic))
-				return FALSE;
-			hdr = (struct isakmp_generic *)(outs->start+offset);
-			if ((len - offset) < ntohs(hdr->isag_length))
-				return FALSE;
-			if ((len - offset) == ntohs(hdr->isag_length)) {
-				hdr->isag_np = np;
-				return TRUE;
-			}
+    u_int8_t *pl = outs->start;
+    size_t left = outs->cur - outs->start;
+
+    passert(left >= NSIZEOF_isakmp_hdr);    /* not even room for isakmp_hdr! */
+    if (left == NSIZEOF_isakmp_hdr) {
+	/* no payloads, just the isakmp_hdr: insert np here */
+	passert(pl[NOFFSETOF_isa_np] == ISAKMP_NEXT_NONE);
+	pl[NOFFSETOF_isa_np] = np;
+    } else {
+	pl += NSIZEOF_isakmp_hdr;       /* skip over isakmp_hdr */
+	left -= NSIZEOF_isakmp_hdr;
+	for (;;) {
+		size_t pllen;
+
+		passert(left >= NSIZEOF_isakmp_generic);
+		pllen = (pl[NOFFSETOF_isag_length] << 8)
+			| pl[NOFFSETOF_isag_length + 1];
+		passert(left >= pllen);
+		if (left == pllen) {
+			/* found last top-level payload */
+			passert(pl[NOFFSETOF_isag_np] == ISAKMP_NEXT_NONE);
+			pl[NOFFSETOF_isag_np] = np;
+			break;  /* done */
+		} else {
+			/* this payload is not the last: scan forward */
+			pl += pllen;
+			left -= pllen;
 		}
 	}
-	return FALSE;
+	}
+	return TRUE;
 }
 
 bool