[PATCH] IPV6_CHECKSUM socket option can corrupt kernel memory

[David Stevens <dlstevens@xxxxxxxxxx>]

I was doing some testing with IPV6_CHECKSUM and started getting memory
corruption (panic when the socket was closed). The bug originally 
manifested
itself as a the "csum" pointer in rawv6_push_pending_frames() pointing 
beyond
the end of the skb data and trashing skb_shinfo(skb) because it didn't 
handle
the case where nr_frags was 1 (and it was being called with an skb like 
that...).

After fixing that, I found that the reason nr_frags was nonzero was 
because
a prior sendto(), which had an invalid offset (for testing) had returned 
an error
but left the packet data in the socket buffer. Subsequent calls wouldn't 
fit in
the existing skb, so they resulted in frag buffers and an invalid csum 
pointer.

I don't know if it is possible, after the fix for the second problem, to 
get an
skb with nonzero nr_frags in rawv6_push_pending_frames() (maybe with
corking? or maybe via MSG_MORE?), but this patch includes support for
non-linear skbs as well as the missing flush on error that caused the 
problem
to begin with.

in-line with mangled whitespace, and attached for applying.

                                                +-DLS

--- linux-2.6.11.7/net/ipv6/raw.c       2005-04-07 11:57:32.000000000 
-0700
+++ linux-2.6.11.7T1/net/ipv6/raw.c     2005-04-12 15:05:00.000000000 
-0700
@@ -465,9 +465,28 @@ static int rawv6_push_pending_frames(str
        if ((skb = skb_peek(&sk->sk_write_queue)) == NULL)
                goto out;
 
-       if (rp->offset + 1 < len)
-               csum = (u16 *)(skb->h.raw + rp->offset);
-       else {
+       if (rp->offset + 1 < len) {
+               if (rp->offset < skb->tail - skb->h.raw - 1) {
+                       csum = (u16 *)(skb->h.raw + rp->offset);
+               } else if (skb_shinfo(skb)->nr_frags) {
+                       int offset = rp->offset;
+                       int i;
+
+                       offset -= skb->tail - skb->h.raw -1;
+                       for (i=0; i<skb_shinfo(skb)->nr_frags; ++i) {
+                               if (offset < 
skb_shinfo(skb)->frags[i].size)
+                                       break;
+                               offset -= skb_shinfo(skb)->frags[i].size;
+                       }
+                       csum = (u16 *)
+ (page_address(skb_shinfo(skb)->frags[i].page) +
+                               skb_shinfo(skb)->frags[i].page_offset + 
offset);
+               } else {
+                       /* "can't happen"; len already checked, but.. */
+                       err = -EINVAL;
+                       goto out;
+               }
+       } else {
                err = -EINVAL;
                goto out;
        }
@@ -771,8 +790,11 @@ back_from_confirm:
 
                if (err)
                        ip6_flush_pending_frames(sk);
-               else if (!(msg->msg_flags & MSG_MORE))
+               else if (!(msg->msg_flags & MSG_MORE)) {
                        err = rawv6_push_pending_frames(sk, &fl, rp, len);
+                       if (err)
+                               ip6_flush_pending_frames(sk);
+               }
        }
 done:
        ip6_dst_store(sk, dst,

rawv6.patch
Description: Binary data