Bus error/invalid alignment on HP-UX (1.1.7)

[pem]

On HP-UX/ia64:

# uname -a
HP-UX hp113-01 B.11.31 U ia64 1093729534 unlimited-user license

this happens in rc_send_server:

Program terminated with signal 10, Bus error.
BUS_ADRALN - Invalid address alignment. Please refer to the following link that helps in handling unaligned data: http://docs.hp.com/en/7730/newhelp0610/pragmas.htm#pragma-pack-ex3
#0  0x4023750:1 in rc_send_server (rh=0x200000004001eb30, 
    data=0x200000007fffd7c0, msg=0x200000007fffd83c "") at sendserver.c:339
339                     auth->length = htons ((unsigned short) total_length);
(gdb) p &retries
$1 = (int *) 0x200000007fff96d4
(gdb) p &send_buffer[0]
$2 = 0x200000007fffb76d "\001\275"
(gdb) p &recv_buffer[0]
$3 = 0x200000007fff976d ""
(gdb) p &vector[0]
$4 = (
    unsigned char *) 0x200000007fff975d "8\016\354\306\245\347\357\330\025\332\343\327\224R!\235"
(gdb) p &secret[0]
$5 = 0x200000007fff972c "testing123"
(gdb) p &secretlen 
$6 = (long unsigned int *) 0x200000007fff96d8
(gdb) p auth
$7 = (struct pw_auth_hdr *) 0x200000007fffb76d
(gdb) 

Note the odd address for auth and send_buffer (and recv_buffer).

A fix has been attached; instead of cast:ing between a misaligned char buffer and a struct, use a union to ensure proper alignment.

... ok, so not attached after all. I get "Something went really wrong, and we can't process that file." no matter in which form I try to upload the patch. Patch can be emailed on request.

[alandekok]

Thanks. You can fork the repo, add the patch, and push it back. Or, if it's a github issue, just attach the patch again in a few minues.

[pem]

Since it's short I'll just add it here:

--- lib/sendserver.c.orig	2018-01-23 10:57:07.861253190 +0100
+++ lib/sendserver.c	2018-01-23 11:19:12.328861458 +0100
@@ -214,6 +214,15 @@
  *
  */
 
+/* Used to avoid casts between char buffers and the header struct since
+** this might cause alignment problems on some architectures.
+ */
+typedef union auth_buf_u
+{
+    AUTH_HDR hdr;
+    char buf[BUFFER_LEN];
+} auth_buf_t;
+
 int rc_send_server (rc_handle *rh, SEND_DATA *data, char *msg)
 {
 	int             sockfd;
@@ -230,8 +239,8 @@
 	size_t			secretlen;
 	char            secret[MAX_SECRET_LENGTH + 1];
 	unsigned char   vector[AUTH_VECTOR_LEN];
-	char            recv_buffer[BUFFER_LEN];
-	char            send_buffer[BUFFER_LEN];
+        auth_buf_t      recv_auth_buf;
+        auth_buf_t      send_auth_buf;
 	int		retries;
 	VALUE_PAIR 	*vp;
 	struct pollfd	pfd;
@@ -313,7 +322,7 @@
 	}
 
 	/* Build a request */
-	auth = (AUTH_HDR *) send_buffer;
+        auth = &send_auth_buf.hdr;
 	auth->code = data->code;
 	auth->id = data->seq_nbr;
 
@@ -345,7 +354,7 @@
 
 	for (;;)
 	{
-		sendto (sockfd, (char *) auth, (unsigned int) total_length, (int) 0,
+		sendto (sockfd, send_auth_buf.buf, (unsigned int) total_length, (int) 0,
 			SA(&sinremote), sizeof (struct sockaddr_in));
 
 		pfd.fd = sockfd;
@@ -383,8 +392,8 @@
 		}
 	}
 	salen = sizeof(sinremote);
-	length = recvfrom (sockfd, (char *) recv_buffer,
-			   (int) sizeof (recv_buffer),
+	length = recvfrom (sockfd, recv_auth_buf.buf,
+			   (int) sizeof (recv_auth_buf),
 			   (int) 0, SA(&sinremote), &salen);
 
 	if (length <= 0)
@@ -396,7 +405,7 @@
 		return ERROR_RC;
 	}
 
-	recv_auth = (AUTH_HDR *)recv_buffer;
+        recv_auth = &recv_auth_buf.hdr;
 
 	if (length < AUTH_HDR_LEN || length < ntohs(recv_auth->length)) {
 		rc_log(LOG_ERR, "rc_send_server: recvfrom: %s:%d: reply is too short",

[pem]

Someone pointed out that this is misusing unions. That's true, in theory, but in practice we know it will work. (The issue is that the standard doesn't guarantee that setting one union field and then reading the other will do what you expect.) If you want to be very strict, I don't think you can guarantee that the AUTH_HDR struct layout will be compact either. (Although we know that it will, in sane implementations.)

Another solution is to simply change char secret[MAX_SECRET_LENGTH + 1]; into char secret[MAX_SECRET_LENGTH + 4]; but I thought that had a kludgy feel to it, and it still relies on casts.

Alternatively, don't use the AUTH_HDR overlay struct, and pack/unpack the header byte by byte instead.

[alandekok]

The simpler way to fix it is to just declare the buffer as uint64_t instead of char. The functions taking char * will happily take a pointer cast from uint64_t *. But the reverse isn't true.