Re: ipv6: handle -EFAULT from skb_copy_bits

From: Hannes Frederic Sowa <redacted>
Date: Wed, 21 Dec 2016 13:41:13 +0100

On Wed, 2016-12-21 at 13:27 +0100, Hannes Frederic Sowa wrote: 

quoted

@@ -555,8 +566,8 @@ static int rawv6_push_pending_frames(struct sock *sk, struct flowi6 *fl6,
                goto out;
 
        offset = rp->offset;
-       total_len = inet_sk(sk)->cork.base.length;
-       if (offset >= total_len - 1) {
+       transport_len = raw6_corked_transport_len(sk);
+       if (offset >= transport_len - 1) {
                err = -EINVAL;
                ip6_flush_pending_frames(sk);
                goto out;

@@ -598,7 +609,7 @@ static int rawv6_push_pending_frames(struct sock *sk, struct flowi6 *fl6,
                tmp_csum = csum_sub(tmp_csum, csum_unfold(csum));
 
        csum = csum_ipv6_magic(&fl6->saddr, &fl6->daddr,
-                              total_len, fl6->flowi6_proto, tmp_csum);
+                              transport_len, fl6->flowi6_proto, tmp_csum);
 

Ops, here we need actually the total_len plus the opt->opt_nflen to
always calculate the correct checksum.

It's a real shame we can't just use skb_transport_offset().  This value
has essentially been calculated for us already.

Also, if we iterate over multiple SKBs in the write queue, don't you have
to redo this calculation for every SKB we iterate over?

Furthermore, what if the user queued up some SKBs in the raw socket
with MSG_MORE, and then changed some of the options for a subsequent
sendmsg() call?

Given all of this, I think the best thing to do is validate the offset
after the queue walks, which is pretty much what Dave Jones's original
patch was doing.

Sigh... well, at least we now understand what's going on here.