linux-ipsec: Re: Structure alignment etc on Alphas :-(

| From: Peter Onion <ponion@srd.bt.co.uk>

| This structure does NOT WORK RIGHT on the alpha....
|
| From pluto/kernel.h
|
| /* espblkrply_edata is from "ipsec_esp.h"
| */
| union fred {
| struct encap_msghdr h;
| struct {
| char dummy[offsetof(struct encap_msghdr, em_dat)];
| struct espblkrply_edata d; /* works for all */
| } u;
| } r_esp, s_esp;

This is an important problem. The fix isn't obvious. Asking for a struct to be unaligned is not the right fix.

Structs are perfectly fine for communication within a machine (using one compiling environment). If all players use the same declarations, the compiler will see to it that the layout is consistent.

The problem in this case is that the interfaces is a little too tricky. For example, in utils/spi.c, the relevant code is in f_blkrply:

        struct espblkrply_edata *ed;

	if((em = malloc(sizeof(struct encap_msghdr) +
		       sizeof(struct espblkrply_edata))) == NULL) {
		fprintf(stderr, "%s: Memory allocation error\n", program_name);
		exit(1);
	}
	ed = (struct espblkrply_edata *)(em->em_dat);

Notice that no explicit care is given to make sure that em->em_dat is suitably aligned. That's fair: you'd expect the definition of struct encap_msghdr would ensure the alignment. But it doesn't. So this will violate certain constraints in the C standard.

Pluto's code makes the same assumption: that em->em_dat is appropriately aligned. But the "failure mode" is different, and so Pluto and spi communicate incompatibly. Since they don't talk to each other, this would seem not to be a problem. But they both talk to the kernel, so at most one can work.

Do you need help?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related lists.freeswan.org Links 1998-fall 1998-spring 1998-summer 1998-winter 1999-winter announce briefs bugs design freeswan-list hipsec ietf-sectest users users-admin Request more information about Pantek

The right fix, I think, is to change the em_dat (pseudo) field to be aligned properly. Here is my suggestion for a fix. Untested! Richard: what do you think?

Hugh Redelmeier
hugh@mimosa.com voice: +1 416 482-8253

• ipsec_netlink.h Tue Dec 1 10:39:56 1998 --- ipsec_netlink.h.fix Thu Dec 3 15:29:12 1998

  ---

• 158,164 **** struct sa_id Said; /* SA ID */ int If; /* enc i/f for input */ int Alg; /* Algorithm to use */ ! __u8 Dat[1]; /* Data */ } Xfm;

                  struct
--- 158,180 ----

  			struct sa_id Said; /* SA ID */
  			int If;		/* enc i/f for input */
  			int Alg;	/* Algorithm to use */
! 
! 			/* The following union is a surrogate for
! 			 * algorithm-specific data.  To insure
! 			 * proper alignment, worst-case fields
! 			 * should be included.  It would be even
! 			 * better to include the types that will
! 			 * actually be used, but they may not be
! 			 * defined for each use of this header.
! 			 * The actual length is expected to be longer
! 			 * than is declared here.  References are normally
! 			 * made using the em_dat macro, as if it were a
! 			 * field name.
! 			 */
! 			union {	/* Data */
! 				__u8 Dat[1];
! 				__u64 Datq[1];	/* maximal alignment (?) */
! 			} u;
  		} Xfm;
  		
  		struct

• 214,220 **** #define em_proto Eu.Xfm.Said.proto #define em_if Eu.Xfm.If #define em_alg Eu.Xfm.Alg ! #define em_dat Eu.Xfm.Dat

  #define em_rel Eu.Rel
  #define emr_dst emr_said.dst
--- 230,236 ----

  #define em_proto	Eu.Xfm.Said.proto
  #define em_if	Eu.Xfm.If
  #define em_alg	Eu.Xfm.Alg
! #define em_dat	Eu.Xfm.u.Dat
  
  #define em_rel	Eu.Rel

================ end ================