netdev - Re: [PATCH net] packet: in packet_snd start writing at link layer allocation

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: <bb77c544-fc52-7984-e421-114a9fd1ac4d@mellanox.com>
Date:   Thu, 24 May 2018 18:07:09 +0300
From:   Tariq Toukan <tariqt@...lanox.com>
To:     David Miller <davem@...emloft.net>, willemdebruijn.kernel@...il.com
Cc:     netdev@...r.kernel.org, eric.dumazet@...il.com, willemb@...gle.com,
        Maor Gottlieb <maorg@...lanox.com>
Subject: Re: [PATCH net] packet: in packet_snd start writing at link layer
 allocation



On 14/05/2018 3:20 AM, David Miller wrote:
> From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
> Date: Fri, 11 May 2018 13:24:25 -0400
> 
>> From: Willem de Bruijn <willemb@...gle.com>
>>
>> Packet sockets allow construction of packets shorter than
>> dev->hard_header_len to accommodate protocols with variable length
>> link layer headers. These packets are padded to dev->hard_header_len,
>> because some device drivers interpret that as a minimum packet size.
>>
>> packet_snd reserves dev->hard_header_len bytes on allocation.
>> SOCK_DGRAM sockets call skb_push in dev_hard_header() to ensure that
>> link layer headers are stored in the reserved range. SOCK_RAW sockets
>> do the same in tpacket_snd, but not in packet_snd.
>>
>> Syzbot was able to send a zero byte packet to a device with massive
>> 116B link layer header, causing padding to cross over into skb_shinfo.
>> Fix this by writing from the start of the llheader reserved range also
>> in the case of packet_snd/SOCK_RAW.
>>
>> Update skb_set_network_header to the new offset. This also corrects
>> it for SOCK_DGRAM, where it incorrectly double counted reserve due to
>> the skb_push in dev_hard_header.
>>
>> Fixes: 9ed988cd5915 ("packet: validate variable length ll headers")
>> Reported-by: syzbot+71d74a5406d02057d559@...kaller.appspotmail.com
>> Signed-off-by: Willem de Bruijn <willemb@...gle.com>
> 
> Applied and queued up for -stable, thanks Willem.
> 

Hi,

One of our regression tests started failing. Once this patch is 
reverted, test passes.

The tests add flow steering rules in the receiver side and in the sender 
side it send the packet with some RAW socket applications. Then received 
side gets completion with error.

Our verification team compared the packets between the stable and the 
broken version, in the broken version we have some extra bytes at the 
end of the packet.

It looks like some bad push to the SKB, maybe the conditional reserved 
addition should be more strict?

Any idea?

Regards,
Tariq