Patchwork [U-Boot] net/dns.c: Fix endian conversion for big-endian in dns command

login
register
mail settings
Submitter Bernhard Kaindl
Date Oct. 16, 2011, 9:59 a.m.
Message ID <1318759162-10523-1-git-send-email-bernhard.kaindl@gmx.net>
Download mbox | patch
Permalink /patch/120019/
State Accepted
Commit 6dc809f40721e5424367b09c8dbfe07da50b8ca9
Headers show

Comments

Bernhard Kaindl - Oct. 16, 2011, 9:59 a.m.
From: Bernhard Kaindl <bernhard.kaindl@thalesgroup.com>

net/dns.c used endian conversion macros wrongly (shorts in reply
were put swapped into CPU, and then ntohs() was used to swap it
back, which broke on big-endian).

Fix this by using the correct linux conversion macro for reading
a unaligned short in network byte order: get_unaligned_be16()
Thanks to Mike Frysinger pointing at the best macro to use.

Tested on big and little endian qemu boards (mips and versatile)

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@thalesgroup.com>
Cc: Pieter Voorthuijsen <pieter.voorthuijsen@prodrive.nl>
Cc: Robin Getz <rgetz@blackfin.uclinux.org>
---
 net/dns.c |   20 ++++++++------------
 1 files changed, 8 insertions(+), 12 deletions(-)
Robin Getz - Oct. 16, 2011, 12:11 p.m.
On Sun 16 Oct 2011 05:59, Bernhard Kaindl pondered:
> From: Bernhard Kaindl <bernhard.kaindl@thalesgroup.com>
> 
> net/dns.c used endian conversion macros wrongly (shorts in reply
> were put swapped into CPU, and then ntohs() was used to swap it
> back, which broke on big-endian).
> 
> Fix this by using the correct linux conversion macro for reading
> a unaligned short in network byte order: get_unaligned_be16()
> Thanks to Mike Frysinger pointing at the best macro to use.
> 
> Tested on big and little endian qemu boards (mips and versatile)

This also tweaks some error messages and comments, which isn't captured in the change log?

Otherwise, Ack from me.

> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@thalesgroup.com>
> Cc: Pieter Voorthuijsen <pieter.voorthuijsen@prodrive.nl>
> Cc: Robin Getz <rgetz@blackfin.uclinux.org>
> ---
>  net/dns.c |   20 ++++++++------------
>  1 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/net/dns.c b/net/dns.c
> index b51d1bd..7a3f1f9 100644
> --- a/net/dns.c
> +++ b/net/dns.c
> @@ -25,6 +25,7 @@
>  #include <common.h>
>  #include <command.h>
>  #include <net.h>
> +#include <asm/unaligned.h>
>  
>  #include "dns.h"
>  
> @@ -109,7 +110,6 @@ DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
>  	int found, stop, dlen;
>  	char IPStr[22];
>  	IPaddr_t IPAddress;
> -	short tmp;
>  
>  
>  	debug("%s\n", __func__);
> @@ -120,14 +120,14 @@ DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
>  		debug("0x%p - 0x%.2x  0x%.2x  0x%.2x  0x%.2x\n",
>  			pkt+i, pkt[i], pkt[i+1], pkt[i+2], pkt[i+3]);
>  
> -	/* We sent 1 query. We want to see more that 1 answer. */
> +	/* We sent one query. We want to have a single answer: */
>  	header = (struct header *) pkt;
>  	if (ntohs(header->nqueries) != 1)
>  		return;
>  
>  	/* Received 0 answers */
>  	if (header->nanswers == 0) {
> -		puts("DNS server returned no answers\n");
> +		puts("DNS: host not found\n");
>  		NetState = NETLOOP_SUCCESS;
>  		return;
>  	}
> @@ -139,9 +139,8 @@ DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
>  		continue;
>  
>  	/* We sent query class 1, query type 1 */
> -	tmp = p[1] | (p[2] << 8);
> -	if (&p[5] > e || ntohs(tmp) != DNS_A_RECORD) {
> -		puts("DNS response was not A record\n");
> +	if (&p[5] > e || get_unaligned_be16(p+1) != DNS_A_RECORD) {
> +		puts("DNS: response was not an A record\n");
>  		NetState = NETLOOP_SUCCESS;
>  		return;
>  	}
> @@ -160,14 +159,12 @@ DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
>  		}
>  		debug("Name (Offset in header): %d\n", p[1]);
>  
> -		tmp = p[2] | (p[3] << 8);
> -		type = ntohs(tmp);
> +		type = get_unaligned_be16(p+2);
>  		debug("type = %d\n", type);
>  		if (type == DNS_CNAME_RECORD) {
>  			/* CNAME answer. shift to the next section */
>  			debug("Found canonical name\n");
> -			tmp = p[10] | (p[11] << 8);
> -			dlen = ntohs(tmp);
> +			dlen = get_unaligned_be16(p+10);
>  			debug("dlen = %d\n", dlen);
>  			p += 12 + dlen;
>  		} else if (type == DNS_A_RECORD) {
> @@ -181,8 +178,7 @@ DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
>  
>  	if (found && &p[12] < e) {
>  
> -		tmp = p[10] | (p[11] << 8);
> -		dlen = ntohs(tmp);
> +		dlen = get_unaligned_be16(p+10);
>  		p += 12;
>  		memcpy(&IPAddress, p, 4);
>
Mike Frysinger - Oct. 16, 2011, 3:14 p.m.
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
Wolfgang Denk - Oct. 23, 2011, 8:57 p.m.
Dear Bernhard Kaindl,

In message <1318759162-10523-1-git-send-email-bernhard.kaindl@gmx.net> you wrote:
> From: Bernhard Kaindl <bernhard.kaindl@thalesgroup.com>
> 
> net/dns.c used endian conversion macros wrongly (shorts in reply
> were put swapped into CPU, and then ntohs() was used to swap it
> back, which broke on big-endian).
> 
> Fix this by using the correct linux conversion macro for reading
> a unaligned short in network byte order: get_unaligned_be16()
> Thanks to Mike Frysinger pointing at the best macro to use.
> 
> Tested on big and little endian qemu boards (mips and versatile)
> 
> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@thalesgroup.com>
> Cc: Pieter Voorthuijsen <pieter.voorthuijsen@prodrive.nl>
> Cc: Robin Getz <rgetz@blackfin.uclinux.org>
> ---
>  net/dns.c |   20 ++++++++------------
>  1 files changed, 8 insertions(+), 12 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

Patch

diff --git a/net/dns.c b/net/dns.c
index b51d1bd..7a3f1f9 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -25,6 +25,7 @@ 
 #include <common.h>
 #include <command.h>
 #include <net.h>
+#include <asm/unaligned.h>
 
 #include "dns.h"
 
@@ -109,7 +110,6 @@  DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
 	int found, stop, dlen;
 	char IPStr[22];
 	IPaddr_t IPAddress;
-	short tmp;
 
 
 	debug("%s\n", __func__);
@@ -120,14 +120,14 @@  DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
 		debug("0x%p - 0x%.2x  0x%.2x  0x%.2x  0x%.2x\n",
 			pkt+i, pkt[i], pkt[i+1], pkt[i+2], pkt[i+3]);
 
-	/* We sent 1 query. We want to see more that 1 answer. */
+	/* We sent one query. We want to have a single answer: */
 	header = (struct header *) pkt;
 	if (ntohs(header->nqueries) != 1)
 		return;
 
 	/* Received 0 answers */
 	if (header->nanswers == 0) {
-		puts("DNS server returned no answers\n");
+		puts("DNS: host not found\n");
 		NetState = NETLOOP_SUCCESS;
 		return;
 	}
@@ -139,9 +139,8 @@  DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
 		continue;
 
 	/* We sent query class 1, query type 1 */
-	tmp = p[1] | (p[2] << 8);
-	if (&p[5] > e || ntohs(tmp) != DNS_A_RECORD) {
-		puts("DNS response was not A record\n");
+	if (&p[5] > e || get_unaligned_be16(p+1) != DNS_A_RECORD) {
+		puts("DNS: response was not an A record\n");
 		NetState = NETLOOP_SUCCESS;
 		return;
 	}
@@ -160,14 +159,12 @@  DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
 		}
 		debug("Name (Offset in header): %d\n", p[1]);
 
-		tmp = p[2] | (p[3] << 8);
-		type = ntohs(tmp);
+		type = get_unaligned_be16(p+2);
 		debug("type = %d\n", type);
 		if (type == DNS_CNAME_RECORD) {
 			/* CNAME answer. shift to the next section */
 			debug("Found canonical name\n");
-			tmp = p[10] | (p[11] << 8);
-			dlen = ntohs(tmp);
+			dlen = get_unaligned_be16(p+10);
 			debug("dlen = %d\n", dlen);
 			p += 12 + dlen;
 		} else if (type == DNS_A_RECORD) {
@@ -181,8 +178,7 @@  DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
 
 	if (found && &p[12] < e) {
 
-		tmp = p[10] | (p[11] << 8);
-		dlen = ntohs(tmp);
+		dlen = get_unaligned_be16(p+10);
 		p += 12;
 		memcpy(&IPAddress, p, 4);