netdev - Re: [PATCH net 2/2] packet: round up linear to header len

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: <CAF=yD-+=bRWn6FZK35vGNrX58KP=H02P0Tg2eJR1is-uDWdSvw@mail.gmail.com>
Date:   Wed, 8 Feb 2017 09:27:04 -0800
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net 2/2] packet: round up linear to header len

On Wed, Feb 8, 2017 at 8:58 AM, Sowmini Varadhan
<sowmini.varadhan@...cle.com> wrote:
> On (02/08/17 08:37), Willem de Bruijn wrote:
>> Date: Wed, 8 Feb 2017 08:37:19 -0800
>> From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
>> To: Sowmini Varadhan <sowmini.varadhan@...cle.com>
>> Cc: Network Development <netdev@...r.kernel.org>, David Miller
>>  <davem@...emloft.net>, Eric Dumazet <eric.dumazet@...il.com>, Dmitry
>>  Vyukov <dvyukov@...gle.com>, Willem de Bruijn <willemb@...gle.com>
>> Subject: Re: [PATCH net 2/2] packet: round up linear to header len
>>
>> On Wed, Feb 8, 2017 at 7:34 AM, Sowmini Varadhan
>> <sowmini.varadhan@...cle.com> wrote:
>> > On (02/07/17 15:57), Willem de Bruijn wrote:
>> >> @@ -2816,8 +2816,9 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>> >>       err = -ENOBUFS;
>> >>       hlen = LL_RESERVED_SPACE(dev);
>> >>       tlen = dev->needed_tailroom;
>> >> -     skb = packet_alloc_skb(sk, hlen + tlen, hlen, len,
>> >> -                            __virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len),
>> >> +     linear = __virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len);
>> >> +     linear = max(linear, min_t(int, len, dev->hard_header_len));
>> >> +     skb = packet_alloc_skb(sk, hlen + tlen, hlen, len, linear,
>> >>                              msg->msg_flags & MSG_DONTWAIT, &err);
>> >
>   :
>> Do you mean the difference that it unconditionally pulls
>> hard_header_len, optionally with zero padding, whereas this new
>> path can check against new min_header_len and thus allows
>> packets shorter than hard_header_len?
>
> yes, maybe it doesnt matter, becaues hard_header_len >= min_header_len
> at all times

The code is not subject to this bug, so I'd rather not touch it in
this fix for stable.

But you raise a good point. This logic is subtle and fragile. It will be good to
deduplicate across packet_snd and tpacket_snd in a follow-up to net-next.