nss_dns: Check for proper A/AAAA address alignment

Message ID 87imuas1os.fsf@oldenburg2.str.redhat.com
State Committed
Headers

Commit Message

Florian Weimer May 16, 2019, 6:18 p.m. UTC
  2019-05-16  Florian Weimer  <fweimer@redhat.com>

	* resolv/nss_dns/dns-host.c (getanswer_r): Be more explicit about
	struct in_addr/struct in6_addr alignment.
  

Comments

Carlos O'Donell May 24, 2019, 4:54 p.m. UTC | #1
On 5/16/19 1:18 PM, Florian Weimer wrote:
> 2019-05-16  Florian Weimer  <fweimer@redhat.com>
> 
> 	* resolv/nss_dns/dns-host.c (getanswer_r): Be more explicit about
> 	struct in_addr/struct in6_addr alignment.
> 

Can we use standard macros to compute alignmnet and differences, it
makes the code more easy to read at a glance.

> diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c
> index 9c15f25f28..9c40051aff 100644
> --- a/resolv/nss_dns/dns-host.c
> +++ b/resolv/nss_dns/dns-host.c
> @@ -947,8 +947,15 @@ getanswer_r (struct resolv_context *ctx,
>  	      linebuflen -= nn;
>  	    }
>  
> -	  linebuflen -= sizeof (align) - ((u_long) bp % sizeof (align));
> -	  bp += sizeof (align) - ((u_long) bp % sizeof (align));
> +	  /* Provide sufficient alignment for both address
> +	     families.  */
> +	  enum { align = 4 };
> +	  _Static_assert ((align % __alignof__ (struct in_addr)) == 0,
> +			  "struct in_addr alignment");
> +	  _Static_assert ((align % __alignof__ (struct in6_addr)) == 0,
> +			  "struct in6_addr alignment");

OK.

> +	  linebuflen -= align - ((u_long) bp % align);
> +	  bp += align - ((u_long) bp % align);

Is the use case common enough to add something to libc-pointer-arith.h
to handle linebuflen adjustment?

e.g.

align_diff = ALIGN_UP_DIFF (bp, align);
linebuflen -= align_diff;
bp += align_diff;

If not then can we still use ALIGN_UP? It makes it immediately
obvious the intent was to align the pointer upwards and adjust
the length of the remaining buffer.

>  
>  	  if (__glibc_unlikely (n > linebuflen))
>  	    goto too_small;
>
  

Patch

diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c
index 9c15f25f28..9c40051aff 100644
--- a/resolv/nss_dns/dns-host.c
+++ b/resolv/nss_dns/dns-host.c
@@ -947,8 +947,15 @@  getanswer_r (struct resolv_context *ctx,
 	      linebuflen -= nn;
 	    }
 
-	  linebuflen -= sizeof (align) - ((u_long) bp % sizeof (align));
-	  bp += sizeof (align) - ((u_long) bp % sizeof (align));
+	  /* Provide sufficient alignment for both address
+	     families.  */
+	  enum { align = 4 };
+	  _Static_assert ((align % __alignof__ (struct in_addr)) == 0,
+			  "struct in_addr alignment");
+	  _Static_assert ((align % __alignof__ (struct in6_addr)) == 0,
+			  "struct in6_addr alignment");
+	  linebuflen -= align - ((u_long) bp % align);
+	  bp += align - ((u_long) bp % align);
 
 	  if (__glibc_unlikely (n > linebuflen))
 	    goto too_small;