IPv6 GRE sending to undefined destinations

Hi,

When using the netlink sendmsg api to pass sockaddr_ll to an IPv6 GRE
interface, strange things happen.

For one, an IPv6 address does not fit into sockaddr_ll.ssl_addr (8 bytes).
Writing 8 0xff bytes into this field causes it to send the packet, but
it also leaks 8 bytes of kernel memory into the packet going out of the
interface.

Now, I have a use case for using this, that is to implement DMVPN/NHRP
over IPv6 GRE, with OpenNHRP for which I need to have a multipoint GRE
connection. With IPv4 this was already implemented with the sll
interface, changing the sll_addr changes the destination of the packet
header.

With the following patch I am able to make it work, but I'm not sure
about the implications of changing the size of the sll_addr buffer.

-- Wilco Baan Hofman



From 9cebcdd74cdbdeda0159aebfb5e64d9b8efc4006 Mon Sep 17 00:00:00 2001
From: Wilco Baan Hofman <wilco@baanhofman.nl>
Date: Fri, 6 Mar 2015 17:25:05 +0100
Subject: [PATCH] Fix ip6_gre sending packets to undefined destinations.

---
 include/uapi/linux/if_packet.h | 2 +-
 net/ipv6/ip6_gre.c             | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

 };
 
 static const struct net_device_ops ip6gre_netdev_ops = {

On Fri, 2015-03-06 at 17:34 +0100, Wilco Baan Hofman wrote:
> Hi,
> 
> When using the netlink sendmsg api to pass sockaddr_ll to an IPv6 GRE
> interface, strange things happen.
> 
> For one, an IPv6 address does not fit into sockaddr_ll.ssl_addr (8 bytes).
> Writing 8 0xff bytes into this field causes it to send the packet, but
> it also leaks 8 bytes of kernel memory into the packet going out of the
> interface.

Oops, that should be fixed first.

> Now, I have a use case for using this, that is to implement DMVPN/NHRP
> over IPv6 GRE, with OpenNHRP for which I need to have a multipoint GRE
> connection. With IPv4 this was already implemented with the sll
> interface, changing the sll_addr changes the destination of the packet
> header.
> 
> With the following patch I am able to make it work, but I'm not sure
> about the implications of changing the size of the sll_addr buffer.

We can't do that as it will break binary compatibility with existing
applications.  It sounds like there is a need for a new, larger
structure but I don't know all the compatibility implications of that.

Ben.

> -- Wilco Baan Hofman
> 
> 
> 
> From 9cebcdd74cdbdeda0159aebfb5e64d9b8efc4006 Mon Sep 17 00:00:00 2001
> From: Wilco Baan Hofman <wilco@baanhofman.nl>
> Date: Fri, 6 Mar 2015 17:25:05 +0100
> Subject: [PATCH] Fix ip6_gre sending packets to undefined destinations.
> 
> ---
>  include/uapi/linux/if_packet.h | 2 +-
>  net/ipv6/ip6_gre.c             | 8 ++++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/if_packet.h b/include/uapi/linux/if_packet.h
> index da2d668..415cbc4 100644
> --- a/include/uapi/linux/if_packet.h
> +++ b/include/uapi/linux/if_packet.h
> @@ -16,7 +16,7 @@ struct sockaddr_ll {
>      unsigned short    sll_hatype;
>      unsigned char    sll_pkttype;
>      unsigned char    sll_halen;
> -    unsigned char    sll_addr[8];
> +    unsigned char    sll_addr[16];
>  };
>  
>  /* Packet types */
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index bc28b7d..c9aee7d 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -1205,8 +1205,16 @@ static int ip6gre_header(struct sk_buff *skb,
> struct net_device *dev,
>      return -t->hlen;
>  }
>  
> +static int ip6gre_header_parse(const struct sk_buff *skb, unsigned char
> *haddr)
> +{
> +    const struct ipv6hdr *ipv6h = (const struct ipv6hdr
> *)skb_mac_header(skb);
> +    memcpy(haddr, &ipv6h->saddr, sizeof(struct in6_addr));
> +    return sizeof(struct in6_addr);
> +}
> +
>  static const struct header_ops ip6gre_header_ops = {
>      .create    = ip6gre_header,
> +    .parse    = ip6gre_header_parse,
>  };
>  
>  static const struct net_device_ops ip6gre_netdev_ops = {

On 07/03/15 18:51, Ben Hutchings wrote:
>> > With the following patch I am able to make it work, but I'm not sure
>> > about the implications of changing the size of the sll_addr buffer.
> We can't do that as it will break binary compatibility with existing
> applications.  It sounds like there is a need for a new, larger
> structure but I don't know all the compatibility implications of that.
>
I'm not so sure. Existing applications will know the old size, 8 bytes
and will only read and set these 8 bytes.
Newer applications can use 16 bytes. It only messes up when newer
headers are used on an older kernel, not the other way around.

The problem is, with the old size, running multipoint tunnels over IPv6
will not work (at least not without an sll netlink interface overhaul).
The NBMA size and interface seems to work, given that the neighbour list
is updated correctly.

$ ip -6 neigh list
fe80::3 dev gre1 lladdr fc:00:00:00:00:00:00:00:00:00:00:00:00:00:00:03
REACHABLE

I can understand not wanting to break newer userspace/older kernel in
this case, though. Any pointers on how to best approach a clean kernel
interface update to make sll netlink work over IPv6 transport?

-- Wilco
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html