[v3] Fix misaligned accesses to fields in HEADER struct defined in <arpa/nameser_compat.h>

Message ID B506D009-1CC9-4EA0-ABDC-CC075C8ADFDF@bell.net
State New, archived
Headers

Commit Message

John David Anglin June 25, 2016, 3:31 p.m. UTC
  On 2016-06-22, at 6:51 AM, Mike Frysinger wrote:

> On 17 Jun 2016 18:52, John David Anglin wrote:
>> --- a/resolv/res_mkquery.c
>> +++ b/resolv/res_mkquery.c
>> @@ -83,6 +83,8 @@
>> # define RANDOM_BITS(Var) { uint64_t v64; HP_TIMING_NOW (v64); Var = v64; }
>> #endif
>> 
>> +typedef HEADER __attribute__ ((aligned(1))) HEADER1;
> 
> could do with a comment above it explaining what this is all about
> 
> should be __aligned__
> 
> bike shed: maybe "UHEADER" is better ?
> 
> code-wise, should this file always be using this variant ?  would
> it be too ugly to do:
> 	typedef ...
> 	#define HEADER HEADER1

The attached patch implements the above suggestions except for res_query.c where
is directly substituted as necessary.

Tested on hppa-unknown-linux-gnu with a trunk build and check.

Please install if okay.

Dave
--
John David Anglin	dave.anglin@bell.net
2016-06-25  John David Anglin  <danglin@gcc.gnu.org>

        [BZ 20243]
        * res_mkquery.c (UHEADER): New typedef derived from HEADER typedef.
	(HEADER): Define.
        * res_send.c: Likewise.
        * res_query.c (UHEADER): New typedef derived from HEADER typedef.
        (__libc_res_nquery): Use UHEADER typedef instead of HEADER for pointers
        requiring byte access.
        (__libc_res_nsearch): Likewise.
  

Patch

diff --git a/resolv/res_mkquery.c b/resolv/res_mkquery.c
index 12f9730..1c4c0bc 100644
--- a/resolv/res_mkquery.c
+++ b/resolv/res_mkquery.c
@@ -83,6 +83,15 @@ 
 # define RANDOM_BITS(Var) { uint64_t v64; HP_TIMING_NOW (v64); Var = v64; }
 #endif
 
+/* The structure HEADER is normally aligned to a word boundary and its
+   fields are accessed using word loads and stores.  We need to access
+   this structure when it is aligned on a byte boundary.  This can cause
+   problems on machines with strict alignment.  So, we create a new
+   typedef to reduce its alignment to one.  This ensures the fields are
+   accessed with byte loads and stores.  */
+typedef HEADER __attribute__ ((__aligned__(1))) UHEADER;
+#define HEADER UHEADER
+
 /*
  * Form all types of queries.
  * Returns the size of the result or -1.
diff --git a/resolv/res_query.c b/resolv/res_query.c
index 944d1a9..ebf0fb9 100644
--- a/resolv/res_query.c
+++ b/resolv/res_query.c
@@ -78,6 +78,14 @@ 
 #include <stdlib.h>
 #include <string.h>
 
+/* The structure HEADER is normally aligned to a word boundary and its
+   fields are accessed using word loads and stores.  We need to access 
+   this structure when it is aligned on a byte boundary.  This can cause
+   problems on machines with strict alignment.  So, we create a new
+   typedef to reduce its alignment to one.  This ensures the fields are
+   accessed with byte loads and stores.  */
+typedef HEADER __attribute__ ((__aligned__(1))) UHEADER;
+
 /* Options.  Leave them on. */
 /* #undef DEBUG */
 
@@ -117,8 +125,8 @@  __libc_res_nquery(res_state statp,
 		  int *resplen2,
 		  int *answerp2_malloced)
 {
-	HEADER *hp = (HEADER *) answer;
-	HEADER *hp2;
+	UHEADER *hp = (UHEADER *) answer;
+	UHEADER *hp2;
 	int n, use_malloc = 0;
 	u_int oflags = statp->_flags;
 
@@ -235,7 +243,7 @@  __libc_res_nquery(res_state statp,
 
 	if (answerp != NULL)
 	  /* __libc_res_nsend might have reallocated the buffer.  */
-	  hp = (HEADER *) *answerp;
+	  hp = (UHEADER *) *answerp;
 
 	/* We simplify the following tests by assigning HP to HP2 or
 	   vice versa.  It is easy to verify that this is the same as
@@ -246,7 +254,7 @@  __libc_res_nquery(res_state statp,
 	  }
 	else
 	  {
-	    hp2 = (HEADER *) *answerp2;
+	    hp2 = (UHEADER *) *answerp2;
 	    if (n < (int) sizeof (HEADER))
 	      {
 	        hp = hp2;
@@ -336,7 +344,7 @@  __libc_res_nsearch(res_state statp,
 		   int *answerp2_malloced)
 {
 	const char *cp, * const *domain;
-	HEADER *hp = (HEADER *) answer;
+	UHEADER *hp = (UHEADER *) answer;
 	char tmp[NS_MAXDNAME];
 	u_int dots;
 	int trailing_dot, ret, saved_herrno;
diff --git a/resolv/res_send.c b/resolv/res_send.c
index 869294f..da075af 100644
--- a/resolv/res_send.c
+++ b/resolv/res_send.c
@@ -110,6 +110,15 @@ 
 #include <kernel-features.h>
 #include <libc-internal.h>
 
+/* The structure HEADER is normally aligned to a word boundary and its
+   fields are accessed using word loads and stores.  We need to access 
+   this structure when it is aligned on a byte boundary.  This can cause
+   problems on machines with strict alignment.  So, we create a new
+   typedef to reduce its alignment to one.  This ensures the fields are
+   accessed with byte loads and stores.  */
+typedef HEADER __attribute__ ((__aligned__(1))) UHEADER;
+#define HEADER UHEADER
+
 #if PACKETSZ > 65536
 #define MAXPACKET       PACKETSZ
 #else