Re: IPv6/sparc64: icmp port unreachable corruption

[Jan Oravec <jan.oravec@xxxxxxx>]

> All of the __skb_{push,pull}() modifications made by icmpv6_send()
> are illegal.  The SKB could be cloned and being inspected by other
> entities in the networking, therefore moving the pointers around
> could cause problems.
> 
> Therefore what we do instead is propagate the:
> 
>       skb->nh.raw - skb->data
> 
> offset into the skb_copy_and_csum_bits() calls.  This is what
> the pre-IPSEC version of the icmpv6 code did.
> 
> When you pass a negative offset into skb_copy_and_csum_bits()
> this means start that many bytes before skb->data

OK, I like your patch more.

You have forgot to decrease 'len' by msg.offset here:


> -     plen = skb->nh.raw - skb->data;
> -     __skb_pull(skb, plen);
> +     msg.skb = skb;
> +     msg.offset = skb->nh.raw - skb->data;
> +
>       len = skb->len;


I've fixed that and tested, here is a working patch:

--- linux/net/ipv6/icmp.c.orig  2003-11-12 16:02:23.000000000 +0100
+++ linux/net/ipv6/icmp.c       2003-11-12 16:03:59.000000000 +0100
@@ -86,15 +86,6 @@
        .flags          =       INET6_PROTO_FINAL,
 };
 
-struct icmpv6_msg {
-       struct icmp6hdr         icmph;
-       struct sk_buff          *skb;
-       int                     offset;
-       struct in6_addr         *daddr;
-       int                     len;
-       __u32                   csum;
-};
-
 static __inline__ int icmpv6_xmit_lock(void)
 {
        local_bh_disable();
@@ -258,11 +249,19 @@
        return err;
 }
 
+struct icmpv6_msg {
+       struct sk_buff  *skb;
+       int             offset;
+};
+
 static int icmpv6_getfrag(void *from, char *to, int offset, int len, int odd, 
struct sk_buff *skb)
 {
-       struct sk_buff *org_skb = (struct sk_buff *)from;
+       struct icmpv6_msg *msg = (struct icmpv6_msg *) from;
+       struct sk_buff *org_skb = msg->skb;
        __u32 csum = 0;
-       csum = skb_copy_and_csum_bits(org_skb, offset, to, len, csum);
+
+       csum = skb_copy_and_csum_bits(org_skb, msg->offset + offset,
+                                     to, len, csum);
        skb->csum = csum_block_add(skb->csum, csum, odd);
        return 0;
 }
@@ -281,9 +280,10 @@
        struct dst_entry *dst;
        struct icmp6hdr tmp_hdr;
        struct flowi fl;
+       struct icmpv6_msg msg;
        int iif = 0;
        int addr_type = 0;
-       int len, plen;
+       int len;
        int hlimit = -1;
        int err = 0;
 
@@ -379,27 +379,29 @@
                        hlimit = dst_metric(dst, RTAX_HOPLIMIT);
        }
 
-       plen = skb->nh.raw - skb->data;
-       __skb_pull(skb, plen);
-       len = skb->len;
+       msg.skb = skb;
+       msg.offset = skb->nh.raw - skb->data;
+
+       len = skb->len - msg.offset;
        len = min_t(unsigned int, len, IPV6_MIN_MTU - sizeof(struct ipv6hdr) 
-sizeof(struct icmp6hdr));
        if (len < 0) {
                if (net_ratelimit())
                        printk(KERN_DEBUG "icmp: len problem\n");
-               __skb_push(skb, plen);
                goto out_dst_release;
        }
 
        idev = in6_dev_get(skb->dev);
 
-       err = ip6_append_data(sk, icmpv6_getfrag, skb, len + sizeof(struct 
icmp6hdr), sizeof(struct icmp6hdr),
-                               hlimit, NULL, &fl, (struct rt6_info*)dst, 
MSG_DONTWAIT);
+       err = ip6_append_data(sk, icmpv6_getfrag, &msg,
+                             len + sizeof(struct icmp6hdr),
+                             sizeof(struct icmp6hdr),
+                             hlimit, NULL, &fl, (struct rt6_info*)dst,
+                             MSG_DONTWAIT);
        if (err) {
                ip6_flush_pending_frames(sk);
                goto out_put;
        }
        err = icmpv6_push_pending_frames(sk, &fl, &tmp_hdr, len + sizeof(struct 
icmp6hdr));
-       __skb_push(skb, plen);
 
        if (type >= ICMPV6_DEST_UNREACH && type <= ICMPV6_PARAMPROB)
                ICMP6_INC_STATS_OFFSET_BH(idev, Icmp6OutDestUnreachs, type - 
ICMPV6_DEST_UNREACH);
@@ -423,6 +425,7 @@
        struct icmp6hdr *icmph = (struct icmp6hdr *) skb->h.raw;
        struct icmp6hdr tmp_hdr;
        struct flowi fl;
+       struct icmpv6_msg msg;
        struct dst_entry *dst;
        int err = 0;
        int hlimit = -1;
@@ -464,7 +467,10 @@
 
        idev = in6_dev_get(skb->dev);
 
-       err = ip6_append_data(sk, icmpv6_getfrag, skb, skb->len + sizeof(struct 
icmp6hdr),
+       msg.skb = skb;
+       msg.offset = 0;
+
+       err = ip6_append_data(sk, icmpv6_getfrag, &msg, skb->len + 
sizeof(struct icmp6hdr),
                                sizeof(struct icmp6hdr), hlimit, NULL, &fl,
                                (struct rt6_info*)dst, MSG_DONTWAIT);
 


Jan