>Number:         1116
>Category:       lib
>Synopsis:       memory xdr streams do unaligned accesses
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people (Library Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jun  5 07:35:02 1995
>Originator:     der Mouse
>Organization:
	Dis-
>Release:        -current as of minutes ago (June 5th)
>Environment:
	Any that gets upset at unaligned accesses
>Description:
	An XDR stream created with xdrmem_create works only if the
	memory block handed to xdrmem_create() is 32bit-aligned,
	because xdrmem_getlong and xdrmem_putlong store through pointer
	casts.  If the block is unaligned, depending on the machine, it
	may (a) work fine, (b) work but with a performance penalty, (c)
	coredump, or (d) silently access memory one to three bytes
	earlier than it should.  (BTW, does NetBSD run on any machines
	that would do (d)?)
>How-To-Repeat:
	This coredumps on NetBSD/sparc.  If the first assignment has
	the +1 removed, it works fine.

	#include <stdlib.h>
	#include <rpc/rpc.h>
	
	void main(void)
	{
	 char *buf;
	 XDR x;
	 double d1;
	 double d2;
	
	 buf = malloc(64) + 1;
	 xdrmem_create(&x,buf,32,XDR_ENCODE);
	 d1 = 123.456789;
	 xdr_double(&x,&d1);
	 xdr_destroy(&x);
	 d2 = 987654.321;
	 xdrmem_create(&x,buf,32,XDR_DECODE);
	 xdr_double(&x,&d2);
	 xdr_destroy(&x);
	 printf("%20.10f -> %20.10f [%20.10f]\n",d1,d2,d1-d2);
	 exit(0);
	}
>Fix:
	(a) Change the docs to note that the memory area must be
	    32bit-aligned,
	(b) Make xdrmem_getlong and xdrmem_putlong do four one-byte
	    loads/stores instead,
	(c) Duplicate enough internals so that there are two versions
	    of xdrmem_{ge,pu}tlong, one for aligned and one for
	    unaligned, and have xdrmem_create choose the correct ops
	    vector depending on the alignment of its argument.

	Personally, I prefer (c).  Here are patches to
	lib/libc/rpc/xdr_mem.c to implement it; with these fixes, the
	above program starts working for me.

--- /sources/current-usr-src/./lib/libc/rpc/xdr_mem.c	Tue May 30 12:37:43 1995
+++ /usr/src/./lib/libc/rpc/xdr_mem.c	Mon Jun  5 10:13:23 1995
@@ -51,29 +51,49 @@
 #include <rpc/xdr.h>
 #include <netinet/in.h>
 
-static bool_t	xdrmem_getlong();
-static bool_t	xdrmem_putlong();
+/* A case could be made that xdrmem_inline should not exist in both aligned
+   and unaligned versions.  However, since the return type is not void *
+   (or char *), it may break badly on architectures that silently ignore
+   the low bits of a pointer.  So we do an unaligned version that fails. */
+
+static bool_t	xdrmem_getlong_aligned();
+static bool_t	xdrmem_putlong_aligned();
+static bool_t	xdrmem_getlong_unaligned();
+static bool_t	xdrmem_putlong_unaligned();
 static bool_t	xdrmem_getbytes();
 static bool_t	xdrmem_putbytes();
 static u_int	xdrmem_getpos(); /* XXX w/64-bit pointers, u_int not enough! */
 static bool_t	xdrmem_setpos();
-static int32_t *xdrmem_inline();
+static int32_t *xdrmem_inline_aligned();
+static int32_t *xdrmem_inline_unaligned();
 static void	xdrmem_destroy();
 
-static struct	xdr_ops xdrmem_ops = {
-	xdrmem_getlong,
-	xdrmem_putlong,
+static struct	xdr_ops xdrmem_ops_aligned = {
+	xdrmem_getlong_aligned,
+	xdrmem_putlong_aligned,
 	xdrmem_getbytes,
 	xdrmem_putbytes,
 	xdrmem_getpos,
 	xdrmem_setpos,
-	xdrmem_inline,
+	xdrmem_inline_aligned,
+	xdrmem_destroy
+};
+
+static struct	xdr_ops xdrmem_ops_unaligned = {
+	xdrmem_getlong_unaligned,
+	xdrmem_putlong_unaligned,
+	xdrmem_getbytes,
+	xdrmem_putbytes,
+	xdrmem_getpos,
+	xdrmem_setpos,
+	xdrmem_inline_unaligned,
 	xdrmem_destroy
 };
 
 /*
  * The procedure xdrmem_create initializes a stream descriptor for a
- * memory buffer.  
+ * memory buffer.  XXX assumes casting pointer to int allows us to
+ * detect whether it's 32bit-aligned. XXX
  */
 void
 xdrmem_create(xdrs, addr, size, op)
@@ -82,9 +102,9 @@
 	u_int size;
 	enum xdr_op op;
 {
-
 	xdrs->x_op = op;
-	xdrs->x_ops = &xdrmem_ops;
+	xdrs->x_ops = (((int)addr)&3) ? &xdrmem_ops_unaligned
+				      : &xdrmem_ops_aligned;
 	xdrs->x_private = xdrs->x_base = addr;
 	xdrs->x_handy = size;
 }
@@ -96,11 +116,10 @@
 }
 
 static bool_t
-xdrmem_getlong(xdrs, lp)
+xdrmem_getlong_aligned(xdrs, lp)
 	register XDR *xdrs;
 	long *lp;
 {
-
 	if ((xdrs->x_handy -= sizeof(int32_t)) < 0)
 		return (FALSE);
 	*lp = (long)ntohl((u_int32_t)(*((int32_t *)(xdrs->x_private))));
@@ -109,11 +128,10 @@
 }
 
 static bool_t
-xdrmem_putlong(xdrs, lp)
+xdrmem_putlong_aligned(xdrs, lp)
 	register XDR *xdrs;
 	long *lp;
 {
-
 	if ((xdrs->x_handy -= sizeof(int32_t)) < 0)
 		return (FALSE);
 	*(int32_t *)xdrs->x_private = (int32_t)htonl((int32_t)(*lp));
@@ -122,6 +140,36 @@
 }
 
 static bool_t
+xdrmem_getlong_unaligned(xdrs, lp)
+	register XDR *xdrs;
+	long *lp;
+{
+	u_int32_t l;
+
+	if ((xdrs->x_handy -= sizeof(int32_t)) < 0)
+		return (FALSE);
+	bcopy(xdrs->x_private,&l,4);
+	*lp = (long)ntohl(l);
+	xdrs->x_private += sizeof(int32_t);
+	return (TRUE);
+}
+
+static bool_t
+xdrmem_putlong_unaligned(xdrs, lp)
+	register XDR *xdrs;
+	long *lp;
+{
+	u_int32_t l;
+
+	if ((xdrs->x_handy -= sizeof(int32_t)) < 0)
+		return (FALSE);
+	l = htonl((u_int32_t)*lp);
+	bcopy(&l,xdrs->x_private,4);
+	xdrs->x_private += sizeof(int32_t);
+	return (TRUE);
+}
+
+static bool_t
 xdrmem_getbytes(xdrs, addr, len)
 	register XDR *xdrs;
 	caddr_t addr;
@@ -174,7 +222,7 @@
 }
 
 static int32_t *
-xdrmem_inline(xdrs, len)
+xdrmem_inline_aligned(xdrs, len)
 	register XDR *xdrs;
 	int len;
 {
@@ -186,4 +234,12 @@
 		xdrs->x_private += len;
 	}
 	return (buf);
+}
+
+static int32_t *
+xdrmem_inline_unaligned(xdrs, len)
+	register XDR *xdrs;
+	int len;
+{
+	return (0);
 }

					der Mouse

			    mouse@collatz.mcrcim.mcgill.edu
>Audit-Trail:
>Unformatted: