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]
Message-ID: <47fca2c7-db7c-0265-d724-38dffc62debe@huawei.com>
Date:   Sat, 15 Apr 2023 10:18:49 +0800
From:   "luwei (O)" <luwei32@...wei.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        Eric Dumazet <edumazet@...gle.com>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "asml.silence@...il.com" <asml.silence@...il.com>,
        "imagedong@...cent.com" <imagedong@...cent.com>,
        "brouer@...hat.com" <brouer@...hat.com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "jbenc@...hat.com" <jbenc@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: 答复: [PATCH net] net: Add check for csum_start in skb_partial_csum_set()


在 2023/4/15 1:20 AM, Willem de Bruijn 写道:
> Willem de Bruijn wrote:
>> luwei (O) wrote:
>>> yes, here is the vnet_hdr:
>>>
>>>      flags: 3
>>>      gso_type: 3
>>>      hdr_len: 23
>>>      gso_size: 58452
>>>      csum_start: 5
>>>      csum_offset: 16
>>>
>>> and the packet:
>>>
>>> | vnet_hdr | mac header | network header | data ... |
>>>
>>>    memcpy((void*)0x20000200,
>>>           "\x03\x03\x02\x00\x54\xe4\x05\x00\x10\x00\x80\x00\x00\x53\xcc\x9c\x2b"
>>>           "\x19\x3b\x00\x00\x00\x89\x4f\x08\x03\x83\x81\x04",
>>>           29);
>>>    *(uint16_t*)0x200000c0 = 0x11;
>>>    *(uint16_t*)0x200000c2 = htobe16(0);
>>>    *(uint32_t*)0x200000c4 = r[3];
>>>    *(uint16_t*)0x200000c8 = 1;
>>>    *(uint8_t*)0x200000ca = 0;
>>>    *(uint8_t*)0x200000cb = 6;
>>>    memset((void*)0x200000cc, 170, 5);
>>>    *(uint8_t*)0x200000d1 = 0;
>>>    memset((void*)0x200000d2, 0, 2);
>>>    syscall(__NR_sendto, r[1], 0x20000200ul, 0xe45ful, 0ul, 0x200000c0ul, 0x14ul);
>> Thanks. So this can happen whenever a packet is injected into the tx
>> path with a virtio_net_hdr.
>>
>> Even if we add bounds checking for the link layer header in pf_packet,
>> it can still point to the network header.
>>
>> If packets are looped to the tx path, skb_pull is common if a packet
>> traverses tunnel devices. But csum_start does not directly matter in
>> the rx path (CHECKSUM_PARTIAL is just seen as CHECKSUM_UNNECESSARY).
>> Until it is forwarded again to the tx path.
>>
>> So the question is which code calls skb_checksum_start_offset on the
>> tx path. Clearly, skb_checksum_help. Also a lot of drivers. Which
>> may cast the signed int return value to an unsigned. Even an u8 in
>> the first driver I spotted (alx).
>>
>> skb_postpull_rcsum anticipates a negative return value, as do other
>> core functions. So it clearly allowed in certain cases. We cannot
>> just bound it.
>>
>> Summary after a long story: an initial investigation, but I don't have
>> a good solution so far. Maybe others have a good suggestiong based on
>> this added context.
> Specific to skb_checksum_help, it appears that skb_checksum will
> work with negative offset just fine.

       In this case maybe not, since it checksums from within the mac 
header, and the mac header

        will be stripped when the rx path checks the checksum.

>
> Perhaps the only issue is that the WARN_ON_ONCE compares signed to
> unsigned, and thus incorrectly interprets a negative offset as
>   >= skb_headlen(skb)
> .

-- 
Best Regards,
Lu Wei

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ