[natty, natty/ti-omap4 CVE 1/1] ipv6: make fragment identifications less predictable
Stefan Bader
stefan.bader at canonical.com
Wed Aug 24 09:53:30 UTC 2011
On 23.08.2011 16:58, Andy Whitcroft wrote:
> [ Backport of upstream commit 87c48fa3b4630905f98268dde838ee43626a060c ]
>
> Fernando Gont reported current IPv6 fragment identification generation
> was not secure, because using a very predictable system-wide generator,
> allowing various attacks.
>
> IPv4 uses inetpeer cache to address this problem and to get good
> performance. We'll use this mechanism when IPv6 inetpeer is stable
> enough in linux-3.1
>
> For the time being, we use jhash on destination address to provide less
> predictable identifications. Also remove a spinlock and use cmpxchg() to
> get better SMP performance.
>
> Reported-by: Fernando Gont <fernando at gont.com.ar>
> Signed-off-by: Eric Dumazet <eric.dumazet at gmail.com>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> Signed-off-by: Greg Kroah-Hartman <gregkh at suse.de>
>
> CVE-2011-2699
> BugLink: http://bugs.launchpad.net/bugs/827685
> Signed-off-by: Andy Whitcroft <apw at canonical.com>
> ---
> include/net/ipv6.h | 12 +-----------
> include/net/transp_v6.h | 2 ++
> net/ipv6/af_inet6.c | 2 ++
> net/ipv6/ip6_output.c | 40 +++++++++++++++++++++++++++++++++++-----
> net/ipv6/udp.c | 2 +-
> 5 files changed, 41 insertions(+), 17 deletions(-)
>
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 96e50e0..24175cf 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -460,17 +460,7 @@ static inline int ipv6_addr_diff(const struct in6_addr *a1, const struct in6_add
> return __ipv6_addr_diff(a1, a2, sizeof(struct in6_addr));
> }
>
> -static __inline__ void ipv6_select_ident(struct frag_hdr *fhdr)
> -{
> - static u32 ipv6_fragmentation_id = 1;
> - static DEFINE_SPINLOCK(ip6_id_lock);
> -
> - spin_lock_bh(&ip6_id_lock);
> - fhdr->identification = htonl(ipv6_fragmentation_id);
> - if (++ipv6_fragmentation_id == 0)
> - ipv6_fragmentation_id = 1;
> - spin_unlock_bh(&ip6_id_lock);
> -}
> +extern void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt);
>
> /*
> * Prototypes exported by ipv6
> diff --git a/include/net/transp_v6.h b/include/net/transp_v6.h
> index 42a0eb6..70eef79 100644
> --- a/include/net/transp_v6.h
> +++ b/include/net/transp_v6.h
> @@ -16,6 +16,8 @@ extern struct proto tcpv6_prot;
>
> struct flowi;
>
> +extern void initialize_hashidentrnd(void);
> +
> /* extention headers */
> extern int ipv6_exthdrs_init(void);
> extern void ipv6_exthdrs_exit(void);
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index 978e80e..ca171b7 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c
> @@ -1081,6 +1081,8 @@ static int __init inet6_init(void)
> goto out;
> }
>
> + initialize_hashidentrnd();
> +
> err = proto_register(&tcpv6_prot, 1);
> if (err)
> goto out;
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 5f8d242..ed0c1a7 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -596,6 +596,35 @@ int ip6_find_1stfragopt(struct sk_buff *skb, u8 **nexthdr)
> return offset;
> }
>
> +static u32 hashidentrnd __read_mostly;
> +#define FID_HASH_SZ 16
> +static u32 ipv6_fragmentation_id[FID_HASH_SZ];
> +
> +void __init initialize_hashidentrnd(void)
> +{
> + get_random_bytes(&hashidentrnd, sizeof(hashidentrnd));
> +}
> +
> +static u32 __ipv6_select_ident(const struct in6_addr *addr)
> +{
> + u32 newid, oldid, hash = jhash2((u32 *)addr, 4, hashidentrnd);
> + u32 *pid = &ipv6_fragmentation_id[hash % FID_HASH_SZ];
> +
> + do {
> + oldid = *pid;
> + newid = oldid + 1;
> + if (!(hash + newid))
> + newid++;
I am trying to understand what hash + newid is supposed to tell. Looking at the
old code there was a test to prevent the id from being 0 when it wrapped. If I
understand this patch correctly there is now multiple ids selected based on the
addresses hash... So why is the above not just
if (!newid)
newid = 1;
?
> + } while (cmpxchg(pid, oldid, newid) != oldid);
> +
> + return hash + newid;
> +}
> +
> +void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt)
> +{
> + fhdr->identification = htonl(__ipv6_select_ident(&rt->rt6i_dst.addr));
> +}
> +
> int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
> {
> struct sk_buff *frag;
> @@ -680,7 +709,7 @@ int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
> skb_reset_network_header(skb);
> memcpy(skb_network_header(skb), tmp_hdr, hlen);
>
> - ipv6_select_ident(fh);
> + ipv6_select_ident(fh, rt);
> fh->nexthdr = nexthdr;
> fh->reserved = 0;
> fh->frag_off = htons(IP6_MF);
> @@ -826,7 +855,7 @@ slow_path:
> fh->nexthdr = nexthdr;
> fh->reserved = 0;
> if (!frag_id) {
> - ipv6_select_ident(fh);
> + ipv6_select_ident(fh, rt);
> frag_id = fh->identification;
> } else
> fh->identification = frag_id;
> @@ -1030,7 +1059,8 @@ static inline int ip6_ufo_append_data(struct sock *sk,
> int getfrag(void *from, char *to, int offset, int len,
> int odd, struct sk_buff *skb),
> void *from, int length, int hh_len, int fragheaderlen,
> - int transhdrlen, int mtu,unsigned int flags)
> + int transhdrlen, int mtu,unsigned int flags,
> + struct rt6_info *rt)
>
> {
> struct sk_buff *skb;
> @@ -1075,7 +1105,7 @@ static inline int ip6_ufo_append_data(struct sock *sk,
> skb_shinfo(skb)->gso_size = (mtu - fragheaderlen -
> sizeof(struct frag_hdr)) & ~7;
> skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
> - ipv6_select_ident(&fhdr);
> + ipv6_select_ident(&fhdr, rt);
> skb_shinfo(skb)->ip6_frag_id = fhdr.identification;
> __skb_queue_tail(&sk->sk_write_queue, skb);
>
> @@ -1231,7 +1261,7 @@ int ip6_append_data(struct sock *sk, int getfrag(void *from, char *to,
>
> err = ip6_ufo_append_data(sk, getfrag, from, length,
> hh_len, fragheaderlen,
> - transhdrlen, mtu, flags);
> + transhdrlen, mtu, flags, rt);
> if (err)
> goto error;
> return 0;
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 119edb6..b6de206 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -1360,7 +1360,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb, int features)
> fptr = (struct frag_hdr *)(skb_network_header(skb) + unfrag_ip6hlen);
> fptr->nexthdr = nexthdr;
> fptr->reserved = 0;
> - ipv6_select_ident(fptr);
> + ipv6_select_ident(fptr, (struct rt6_info *)skb_dst(skb));
>
> /* Fragment the skb. ipv6 header and the remaining fields of the
> * fragment header are updated in ipv6_gso_segment()
More information about the kernel-team
mailing list