netdev - Re: [PATCH RFC net-next] packet: always ensure that we pass hard_header_len bytes in skb

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:   Fri, 27 Jan 2017 09:37:14 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:     David Miller <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH RFC net-next] packet: always ensure that we pass
 hard_header_len bytes in skb_headlen() to the driver

On Thu, Jan 26, 2017 at 9:08 PM, Sowmini Varadhan
<sowmini.varadhan@...cle.com> wrote:
> On (01/26/17 19:08), Willem de Bruijn wrote:
>>
>> Thanks for the context. ax25_addr_parse doesn't adjust length, it only
>> verifies that the contents of the variable length header matches
>> protocol spec. I don't think that it or the .validate callback have to
>> be modified to return length.
>
> Yes, I noticed that too, but my reading of ax25_addr_parse
> was that it checks to see that a sane L2 header has been
> passed in, and if that (sane-header) is the case, it
> returns pointer to the start of data. Thus the returned
> (non-null) pointer minus start should tell you the "real"
> header length- is my understanding correct?

Yes. ax25_validate_header reads up to len bytes to parse the header,
so it is smaller than or equal to len or the function returns false.
It is not necessary to check whether the return value exceeds the len
passed to dev_validate_header.

The immediate problem you were facing is that dev_validate_header
accepts values smaller than hard_header_len even for protocols with
fixed header lengths.

This is a consequence of that CAP_SYS_RAWIO branch. Without it,
dev_validate_header would have correctly dropped your packet. That
branch was added because there are tests that explicitly test bad
input. Ideally, it would be behind sysctl and static key, but doing so
might start failing active tests.

See also this discussion

  Sending short raw packets using sendmsg() broke
  https://www.mail-archive.com/netdev@vger.kernel.org/msg99081.html