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] [day] [month] [year] [list]

Message-ID: <CAF=yD-LiptFZCM3akz8-uMA=UH1sTbsXtWQp1=xT52YaWrfqUA@mail.gmail.com>
Date:   Tue, 7 Feb 2017 12:51:54 -0800
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     Sowmini Varadhan <sowmini.varadhan@...cle.com>,
        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 Mon, Jan 30, 2017 at 8:41 AM, David Miller <davem@...emloft.net> wrote:
> From: Sowmini Varadhan <sowmini.varadhan@...cle.com>
> Date: Mon, 30 Jan 2017 11:26:03 -0500
>
>> On (01/27/17 19:19), Willem de Bruijn wrote:
>>> > other than ax25, are there variable length header protocols out there
>>> > without ->validate, and which need the CAP_RAW_SYSIO branch?
>>>
>>> I don't know. An exhaustive search of protocols (by header_ops) may be
>>> needed to say for sure.
>>>
>>> If there are none, then the solution indeed is quite simple.
>>
>>
>> I tried to start that exhaustive search, and it can be quite daunting:
>> if you are doing this by just code-inspection, it's easy to get
>> it wrong.. I havent quite given up yet, but it may be simpler to have
>> the drivers support some defensive code against bogus skb's in the
>> Tx path (the drivers will know, for sure, what's the min non-paged
>> len they need anyway).
>
> I think it's easier to audit all the header_ops than to add defensive
> code to 500+ drivers.

This issue came up again in a slightly different context. I scanned
the implementations of header_ops. Variable length link layer headers
are quite common. Not necessarily as malleable as ax25, but in
the form of fixed headers with a limited set of optional extensions,
such as ipgre. For this reason, adding ->validate implementations for
each is infeasible, especially for a patch to net.

I think that the right approach is to finally introduce an explicit
dev->min_header_length field, and initialize that at least for
Ethernet and loopback.

I will send that and a related patch.