netdev - Re: [PATCH] xfrm: use gre key as flow upper protocol info

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 23 Nov 2010 16:03:45 +0200
From:	Timo Teräs <timo.teras@....fi>
To:	David Miller <davem@...emloft.net>
CC:	netdev@...r.kernel.org, herbert@...dor.apana.org.au
Subject: Re: [PATCH] xfrm: use gre key as flow upper protocol info

On 11/15/2010 08:43 PM, David Miller wrote:
> From: Timo Teräs <timo.teras@....fi>
> Date: Wed,  3 Nov 2010 16:41:38 +0200
> 
>> The GRE Key field is intended to be used for identifying an individual
>> traffic flow within a tunnel. It is useful to be able to have XFRM
>> policy selector matches to have different policies for different
>> GRE tunnels.
>>
>> Signed-off-by: Timo Teräs <timo.teras@....fi>
> 
> I'll apply this to net-next-2.6, thanks.

Hmm.. I tested this with using the "ip xfrm" sport and dport manually
(without doing the actual userland support for this), and checking it in
kernel with printk's in various places that the stuff matches. In these
tests I checked the sport/dport by hand and apparently messed up the
byte order.

Now that I'm writing the GRE support for "ip xfrm" I think that missed
two htons() calls.

I was confused if xfrm_flowi_{s|d}port was supposed to return host or
net byte order for non-TCP/UDP packets.

I was under the assumption that host byte order since case IPPROTO_ICMP
swaps the byte order. But it would appear that the fl->fl_icmp_* is
actually host order and it's turned to network order; this is also
implied by using htons instead of ntohs. Since I decided to keep
fl_gre_key in network order, the return value would now be inconsistent,
and make userland abi endianess dependent.

I'll follow up with iproute2 patch soon.

So we probably would need to do:

xfrm: fix gre key endianess

fl->fl_gre_key is network byte order contrary to fl->fl_icmp_*.
Make xfrm_flowi_{s|d}port return network byte order values for gre
key too.

Signed-off-by: Timo Teräs <timo.teras@....fi>

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 1a57ff9..916ac47 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -806,7 +806,7 @@ __be16 xfrm_flowi_sport(struct flowi *fl)
 		port = htons(fl->fl_mh_type);
 		break;
 	case IPPROTO_GRE:
-		port = htonl(fl->fl_gre_key) >> 16;
+		port = htons(ntohl(fl->fl_gre_key) >> 16);
 		break;
 	default:
 		port = 0;	/*XXX*/
@@ -830,7 +830,7 @@ __be16 xfrm_flowi_dport(struct flowi *fl)
 		port = htons(fl->fl_icmp_code);
 		break;
 	case IPPROTO_GRE:
-		port = htonl(fl->fl_gre_key) & 0xffff;
+		port = htons(ntohl(fl->fl_gre_key) & 0xffff);
 		break;
 	default:
 		port = 0;	/*XXX*/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html