netdev - Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 24 Mar 2016 22:33:08 -0700
From:	Cong Wang <xiyou.wangcong@...il.com>
To:	Ben Greear <greearb@...delatech.com>
Cc:	Vijay Pandurangan <vijayp@...ayp.ca>,
	netdev <netdev@...r.kernel.org>, Evan Jones <ej@...njones.ca>,
	Cong Wang <cwang@...pensource.com>
Subject: Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."

On Thu, Mar 24, 2016 at 10:13 PM, Ben Greear <greearb@...delatech.com> wrote:
>
>
> On 03/24/2016 10:06 PM, Cong Wang wrote:
>>
>> On Thu, Mar 24, 2016 at 9:34 PM, Ben Greear <greearb@...delatech.com>
>> wrote:
>>>
>>>
>>>
>>> On 03/24/2016 06:44 PM, Vijay Pandurangan wrote:
>>>>
>>>>
>>>> Oops, I think my last email didn't go through due to an inadvertent
>>>> html attachment from my phone mail client.
>>>>
>>>> Can you send us a copy of a packet you're sending and/or confirm that
>>>> the IP and UDP4 checksums are set correctly in the packet?
>>>>
>>>> If those are set right, I think we need to read through the networking
>>>> code again to see why this is broken...
>>>
>>>
>>>
>>> Wireshark decodes the packet as having no checksum errors.
>>>
>>> I think the contents of the packet is correct, but the 'ip_summed'
>>> field is set incorrectly to 'NONE' when transmitting on a raw packet
>>> socket.
>>
>>
>> Yeah, these bugs are all due to the different interpretations of
>> ip_summed on TX path and RX path. I think the following patch
>> should work, if the comments don't mislead me. Could you give
>> it a try?
>>
>> For the long term, we need to unify the meaning of ip_summed
>> on TX path and RX path, or at least translate it in skb_scrub_packet().
>
>
> I can test this tomorrow, but I think it will not work.  I'm not sending raw
> IP frames, I'm sending full ethernet frames.  Socket is PF_PACKET, SOCK_RAW.
>
> Your patch may still be useful for others though?

Here we go:

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 1ecfa71..ab66080 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1925,6 +1925,7 @@ static int packet_sendmsg_spkt(struct socket
*sock, struct msghdr *msg,
                goto out_unlock;
        }

+       skb->ip_summed = CHECKSUM_UNNECESSARY;
        skb->protocol = proto;
        skb->dev = dev;
        skb->priority = sk->sk_priority;
@@ -2496,6 +2497,7 @@ static int tpacket_fill_skb(struct packet_sock
*po, struct sk_buff *skb,

        ph.raw = frame;

+       skb->ip_summed = CHECKSUM_UNNECESSARY;
        skb->protocol = proto;
        skb->dev = dev;
        skb->priority = po->sk.sk_priority;
@@ -2805,6 +2807,7 @@ static struct sk_buff *packet_alloc_skb(struct
sock *sk, size_t prepad,
        skb_put(skb, linear);
        skb->data_len = len - linear;
        skb->len += len - linear;
+       skb->ip_summed = CHECKSUM_UNNECESSARY;

        return skb;
 }


Thanks for testing!